diff --git a/src/Umbraco.Core/Constants-Conventions.cs b/src/Umbraco.Core/Constants-Conventions.cs index 16c63b4c02..cb34901e6c 100644 --- a/src/Umbraco.Core/Constants-Conventions.cs +++ b/src/Umbraco.Core/Constants-Conventions.cs @@ -1,3 +1,5 @@ +using System; + namespace Umbraco.Cms.Core { public static partial class Constants @@ -187,43 +189,49 @@ namespace Umbraco.Cms.Core /// /// Property alias for the Approved boolean of a Member /// + [Obsolete("IsApproved is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string IsApproved = "umbracoMemberApproved"; - + [Obsolete("Use the stateApproved translation in the user area instead, scheduled for removal in V11")] public const string IsApprovedLabel = "Is Approved"; /// /// Property alias for the Locked out boolean of a Member /// + [Obsolete("IsLockedOut is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string IsLockedOut = "umbracoMemberLockedOut"; - + [Obsolete("Use the stateLockedOut translation in the user area instead, scheduled for removal in V11")] public const string IsLockedOutLabel = "Is Locked Out"; /// /// Property alias for the last date the Member logged in /// + [Obsolete("LastLoginDate is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string LastLoginDate = "umbracoMemberLastLogin"; - + [Obsolete("Use the lastLogin translation in the user area instead, scheduled for removal in V11")] public const string LastLoginDateLabel = "Last Login Date"; /// /// Property alias for the last date a Member changed its password /// + [Obsolete("LastPasswordChangeDate is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string LastPasswordChangeDate = "umbracoMemberLastPasswordChangeDate"; - + [Obsolete("Use the lastPasswordChangeDate translation in the user area instead, scheduled for removal in V11")] public const string LastPasswordChangeDateLabel = "Last Password Change Date"; /// /// Property alias for the last date a Member was locked out /// + [Obsolete("LastLockoutDate is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string LastLockoutDate = "umbracoMemberLastLockoutDate"; - + [Obsolete("Use the lastLockoutDate translation in the user area instead, scheduled for removal in V11")] public const string LastLockoutDateLabel = "Last Lockout Date"; /// /// Property alias for the number of failed login attempts /// + [Obsolete("FailedPasswordAttempts is no longer property data, access the property directly on IMember instead, scheduled for removal in V11")] public const string FailedPasswordAttempts = "umbracoMemberFailedPasswordAttempts"; - + [Obsolete("Use the failedPasswordAttempts translation in the user area instead, scheduled for removal in V11")] public const string FailedPasswordAttemptsLabel = "Failed Password Attempts"; /// diff --git a/src/Umbraco.Core/ConventionsHelper.cs b/src/Umbraco.Core/ConventionsHelper.cs index f5e0b168b9..2f9203ef92 100644 --- a/src/Umbraco.Core/ConventionsHelper.cs +++ b/src/Umbraco.Core/ConventionsHelper.cs @@ -11,64 +11,16 @@ namespace Umbraco.Cms.Core { { Constants.Conventions.Member.Comments, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.TextArea, ValueStorageType.Ntext, true, + new PropertyType( + shortStringHelper, + Constants.PropertyEditors.Aliases.TextArea, + ValueStorageType.Ntext, + true, Constants.Conventions.Member.Comments) { - Name = Constants.Conventions.Member.CommentsLabel + Name = Constants.Conventions.Member.CommentsLabel, } }, - { - Constants.Conventions.Member.FailedPasswordAttempts, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer, true, - Constants.Conventions.Member.FailedPasswordAttempts) - { - Name = Constants.Conventions.Member.FailedPasswordAttemptsLabel, - DataTypeId = Constants.DataTypes.LabelInt - } - }, - { - Constants.Conventions.Member.IsApproved, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Boolean, ValueStorageType.Integer, true, - Constants.Conventions.Member.IsApproved) - { - Name = Constants.Conventions.Member.IsApprovedLabel - } - }, - { - Constants.Conventions.Member.IsLockedOut, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Boolean, ValueStorageType.Integer, true, - Constants.Conventions.Member.IsLockedOut) - { - Name = Constants.Conventions.Member.IsLockedOutLabel - } - }, - { - Constants.Conventions.Member.LastLockoutDate, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, true, - Constants.Conventions.Member.LastLockoutDate) - { - Name = Constants.Conventions.Member.LastLockoutDateLabel, - DataTypeId = Constants.DataTypes.LabelDateTime - } - }, - { - Constants.Conventions.Member.LastLoginDate, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, true, - Constants.Conventions.Member.LastLoginDate) - { - Name = Constants.Conventions.Member.LastLoginDateLabel, - DataTypeId = Constants.DataTypes.LabelDateTime - } - }, - { - Constants.Conventions.Member.LastPasswordChangeDate, - new PropertyType(shortStringHelper, Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, true, - Constants.Conventions.Member.LastPasswordChangeDate) - { - Name = Constants.Conventions.Member.LastPasswordChangeDateLabel, - DataTypeId = Constants.DataTypes.LabelDateTime - } - } }; } } diff --git a/src/Umbraco.Core/Models/ContentEditing/MemberDisplay.cs b/src/Umbraco.Core/Models/ContentEditing/MemberDisplay.cs index 809e47e073..198321a483 100644 --- a/src/Umbraco.Core/Models/ContentEditing/MemberDisplay.cs +++ b/src/Umbraco.Core/Models/ContentEditing/MemberDisplay.cs @@ -24,6 +24,12 @@ namespace Umbraco.Cms.Core.Models.ContentEditing [DataMember(Name = "email")] public string Email { get; set; } + [DataMember(Name = "isLockedOut")] + public bool IsLockedOut { get; set; } + + [DataMember(Name = "isApproved")] + public bool IsApproved { get; set; } + //[DataMember(Name = "membershipScenario")] //public MembershipScenario MembershipScenario { get; set; } diff --git a/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs b/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs index 6b5c0096a0..a2e65e8275 100644 --- a/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs +++ b/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs @@ -31,15 +31,11 @@ namespace Umbraco.Cms.Core.Models.ContentEditing /// public string Comments => GetPropertyValue(Constants.Conventions.Member.Comments); - /// - /// Returns the value from the IsLockedOut property - /// - public bool IsLockedOut => GetPropertyValue(Constants.Conventions.Member.IsLockedOut); + [DataMember(Name = "isLockedOut")] + public bool IsLockedOut { get; set; } - /// - /// Returns the value from the IsApproved property - /// - public bool IsApproved => GetPropertyValue(Constants.Conventions.Member.IsApproved); + [DataMember(Name = "isApproved")] + public bool IsApproved { get; set; } private T GetPropertyValue(string alias) { diff --git a/src/Umbraco.Core/Models/ContentEditing/UserDisplay.cs b/src/Umbraco.Core/Models/ContentEditing/UserDisplay.cs index f16ad854e6..a147dcba34 100644 --- a/src/Umbraco.Core/Models/ContentEditing/UserDisplay.cs +++ b/src/Umbraco.Core/Models/ContentEditing/UserDisplay.cs @@ -64,11 +64,11 @@ namespace Umbraco.Cms.Core.Models.ContentEditing [DataMember(Name = "lastLockoutDate")] [ReadOnly(true)] - public DateTime LastLockoutDate { get; set; } + public DateTime? LastLockoutDate { get; set; } [DataMember(Name = "lastPasswordChangeDate")] [ReadOnly(true)] - public DateTime LastPasswordChangeDate { get; set; } + public DateTime? LastPasswordChangeDate { get; set; } [DataMember(Name = "createDate")] [ReadOnly(true)] diff --git a/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs b/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs index d8ac8d635d..3ddd5e66b9 100644 --- a/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs +++ b/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs @@ -65,13 +65,6 @@ namespace Umbraco.Cms.Core.Models.Mapping var resolved = base.Map(source, context); - // IMember.IsLockedOut can't be set to true, so make it readonly when that's the case (you can only unlock) - var isLockedOutProperty = resolved.SelectMany(x => x.Properties).FirstOrDefault(x => x.Alias == Constants.Conventions.Member.IsLockedOut); - if (isLockedOutProperty?.Value != null && isLockedOutProperty.Value.ToString() != "1") - { - isLockedOutProperty.Readonly = true; - } - return resolved; } @@ -189,7 +182,7 @@ namespace Umbraco.Cms.Core.Models.Mapping var properties = new List { GetLoginProperty(member, _localizedTextService), - new ContentPropertyDisplay + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email", Label = _localizedTextService.Localize("general","email"), @@ -197,7 +190,7 @@ namespace Umbraco.Cms.Core.Models.Mapping View = "email", Validation = { Mandatory = true } }, - new ContentPropertyDisplay + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password", Label = _localizedTextService.Localize(null,"password"), @@ -209,7 +202,7 @@ namespace Umbraco.Cms.Core.Models.Mapping View = "changepassword", Config = GetPasswordConfig(member) // Initialize the dictionary with the configuration from the default membership provider }, - new ContentPropertyDisplay + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}membergroup", Label = _localizedTextService.Localize("content","membergroup"), @@ -218,9 +211,80 @@ namespace Umbraco.Cms.Core.Models.Mapping Config = new Dictionary { { "IsRequired", true } + }, + }, + + // These properties used to live on the member as property data, defaulting to sensitive, so we set them to sensitive here too + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}failedPasswordAttempts", + Label = _localizedTextService.Localize("user", "failedPasswordAttempts"), + Value = member.FailedPasswordAttempts, + View = "readonlyvalue", + IsSensitive = true, + }, + + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}approved", + Label = _localizedTextService.Localize("user", "stateApproved"), + Value = member.IsApproved, + View = "boolean", + IsSensitive = true, + Readonly = false, + }, + + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lockedOut", + Label = _localizedTextService.Localize("user", "stateLockedOut"), + Value = member.IsLockedOut, + View = "boolean", + IsSensitive = true, + Readonly = !member.IsLockedOut, // IMember.IsLockedOut can't be set to true, so make it readonly when that's the case (you can only unlock) + }, + + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate", + Label = _localizedTextService.Localize("user", "lastLockoutDate"), + Value = member.LastLockoutDate?.ToString(), + View = "readonlyvalue", + IsSensitive = true, + }, + + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLoginDate", + Label = _localizedTextService.Localize("user", "lastLogin"), + Value = member.LastLoginDate?.ToString(), + View = "readonlyvalue", + IsSensitive = true, + }, + + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastPasswordChangeDate", + Label = _localizedTextService.Localize("user", "lastPasswordChangeDate"), + Value = member.LastPasswordChangeDate?.ToString(), + View = "readonlyvalue", + IsSensitive = true, + }, + }; + + if (_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.HasAccessToSensitiveData() is false) + { + // Current user doesn't have access to sensitive data so explicitly set the views and remove the value from sensitive data + foreach (var property in properties) + { + if (property.IsSensitive) + { + property.Value = null; + property.View = "sensitivevalue"; + property.Readonly = true; } } - }; + } return properties; } diff --git a/src/Umbraco.Core/Models/Mapping/UserMapDefinition.cs b/src/Umbraco.Core/Models/Mapping/UserMapDefinition.cs index e4d101ff06..d5a76324dd 100644 --- a/src/Umbraco.Core/Models/Mapping/UserMapDefinition.cs +++ b/src/Umbraco.Core/Models/Mapping/UserMapDefinition.cs @@ -294,7 +294,7 @@ namespace Umbraco.Cms.Core.Models.Mapping target.Id = source.Id; target.Key = source.Key; target.LastLockoutDate = source.LastLockoutDate; - target.LastLoginDate = source.LastLoginDate == default ? null : (DateTime?)source.LastLoginDate; + target.LastLoginDate = source.LastLoginDate == default(DateTime) ? null : (DateTime?)source.LastLoginDate; target.LastPasswordChangeDate = source.LastPasswordChangeDate; target.Name = source.Name; target.Navigation = CreateUserEditorNavigation(); diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 146f19bf36..447b1816f2 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -21,6 +21,12 @@ namespace Umbraco.Cms.Core.Models private string _passwordConfig; private DateTime? _emailConfirmedDate; private string _securityStamp; + private int _failedPasswordAttempts; + private bool _isApproved; + private bool _isLockedOut; + private DateTime? _lastLockoutDate; + private DateTime? _lastLoginDate; + private DateTime? _lastPasswordChangeDate; /// /// Initializes a new instance of the class. @@ -281,41 +287,13 @@ namespace Umbraco.Cms.Core.Models } /// - /// Gets or sets a boolean indicating whether the Member is approved + /// Gets or sets a value indicating whether the Member is approved /// - /// - /// Alias: umbracoMemberApproved - /// Part of the standard properties collection. - /// [DataMember] public bool IsApproved { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsApproved, nameof(IsApproved), - //This is the default value if the prop is not found - true); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.IsApproved].GetValue() == null) - return true; - var tryConvert = Properties[Constants.Conventions.Member.IsApproved].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - //if the property exists but it cannot be converted, we will assume true - return true; - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.IsApproved, - nameof(IsApproved)) == false) - return; - - Properties[Constants.Conventions.Member.IsApproved].SetValue(value); - } + get => _isApproved; + set => SetPropertyValueAndDetectChanges(value, ref _isApproved, nameof(IsApproved)); } /// @@ -328,30 +306,8 @@ namespace Umbraco.Cms.Core.Models [DataMember] public bool IsLockedOut { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsLockedOut, nameof(IsLockedOut), false); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.IsLockedOut].GetValue() == null) - return false; - var tryConvert = Properties[Constants.Conventions.Member.IsLockedOut].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - return false; - // TODO: Use TryConvertTo instead - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.IsLockedOut, - nameof(IsLockedOut)) == false) - return; - - Properties[Constants.Conventions.Member.IsLockedOut].SetValue(value); - } + get => _isLockedOut; + set => SetPropertyValueAndDetectChanges(value, ref _isLockedOut, nameof(IsLockedOut)); } /// @@ -362,32 +318,10 @@ namespace Umbraco.Cms.Core.Models /// Part of the standard properties collection. /// [DataMember] - public DateTime LastLoginDate + public DateTime? LastLoginDate { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLoginDate, nameof(LastLoginDate), default(DateTime)); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.LastLoginDate].GetValue() == null) - return default(DateTime); - var tryConvert = Properties[Constants.Conventions.Member.LastLoginDate].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - return default(DateTime); - // TODO: Use TryConvertTo instead - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.LastLoginDate, - nameof(LastLoginDate)) == false) - return; - - Properties[Constants.Conventions.Member.LastLoginDate].SetValue(value); - } + get => _lastLoginDate; + set => SetPropertyValueAndDetectChanges(value, ref _lastLoginDate, nameof(LastLoginDate)); } /// @@ -398,32 +332,10 @@ namespace Umbraco.Cms.Core.Models /// Part of the standard properties collection. /// [DataMember] - public DateTime LastPasswordChangeDate + public DateTime? LastPasswordChangeDate { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastPasswordChangeDate, nameof(LastPasswordChangeDate), default(DateTime)); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.LastPasswordChangeDate].GetValue() == null) - return default(DateTime); - var tryConvert = Properties[Constants.Conventions.Member.LastPasswordChangeDate].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - return default(DateTime); - // TODO: Use TryConvertTo instead - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.LastPasswordChangeDate, - nameof(LastPasswordChangeDate)) == false) - return; - - Properties[Constants.Conventions.Member.LastPasswordChangeDate].SetValue(value); - } + get => _lastPasswordChangeDate; + set => SetPropertyValueAndDetectChanges(value, ref _lastPasswordChangeDate, nameof(LastPasswordChangeDate)); } /// @@ -434,32 +346,10 @@ namespace Umbraco.Cms.Core.Models /// Part of the standard properties collection. /// [DataMember] - public DateTime LastLockoutDate + public DateTime? LastLockoutDate { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLockoutDate, nameof(LastLockoutDate), default(DateTime)); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.LastLockoutDate].GetValue() == null) - return default(DateTime); - var tryConvert = Properties[Constants.Conventions.Member.LastLockoutDate].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - return default(DateTime); - // TODO: Use TryConvertTo instead - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.LastLockoutDate, - nameof(LastLockoutDate)) == false) - return; - - Properties[Constants.Conventions.Member.LastLockoutDate].SetValue(value); - } + get => _lastLockoutDate; + set => SetPropertyValueAndDetectChanges(value, ref _lastLockoutDate, nameof(LastLockoutDate)); } /// @@ -473,30 +363,8 @@ namespace Umbraco.Cms.Core.Models [DataMember] public int FailedPasswordAttempts { - get - { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.FailedPasswordAttempts, nameof(FailedPasswordAttempts), 0); - if (a.Success == false) - return a.Result; - if (Properties[Constants.Conventions.Member.FailedPasswordAttempts].GetValue() == null) - return default(int); - var tryConvert = Properties[Constants.Conventions.Member.FailedPasswordAttempts].GetValue().TryConvertTo(); - if (tryConvert.Success) - { - return tryConvert.Result; - } - return default(int); - // TODO: Use TryConvertTo instead - } - set - { - if (WarnIfPropertyTypeNotFoundOnSet( - Constants.Conventions.Member.FailedPasswordAttempts, - nameof(FailedPasswordAttempts)) == false) - return; - - Properties[Constants.Conventions.Member.FailedPasswordAttempts].SetValue(value); - } + get => _failedPasswordAttempts; + set => SetPropertyValueAndDetectChanges(value, ref _failedPasswordAttempts, nameof(FailedPasswordAttempts)); } /// diff --git a/src/Umbraco.Core/Models/Membership/IMembershipUser.cs b/src/Umbraco.Core/Models/Membership/IMembershipUser.cs index 29a6bf4cdb..91eadd9a9c 100644 --- a/src/Umbraco.Core/Models/Membership/IMembershipUser.cs +++ b/src/Umbraco.Core/Models/Membership/IMembershipUser.cs @@ -25,9 +25,9 @@ namespace Umbraco.Cms.Core.Models.Membership string Comments { get; set; } bool IsApproved { get; set; } bool IsLockedOut { get; set; } - DateTime LastLoginDate { get; set; } - DateTime LastPasswordChangeDate { get; set; } - DateTime LastLockoutDate { get; set; } + DateTime? LastLoginDate { get; set; } + DateTime? LastPasswordChangeDate { get; set; } + DateTime? LastLockoutDate { get; set; } /// /// Gets or sets the number of failed password attempts. diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 5525532f9d..826899eae3 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -116,9 +116,9 @@ namespace Umbraco.Cms.Core.Models.Membership private bool _isApproved; private bool _isLockedOut; private string _language; - private DateTime _lastPasswordChangedDate; - private DateTime _lastLoginDate; - private DateTime _lastLockoutDate; + private DateTime? _lastPasswordChangedDate; + private DateTime? _lastLoginDate; + private DateTime? _lastLockoutDate; //Custom comparer for enumerable private static readonly DelegateEqualityComparer> IntegerEnumerableComparer = @@ -184,21 +184,21 @@ namespace Umbraco.Cms.Core.Models.Membership } [IgnoreDataMember] - public DateTime LastLoginDate + public DateTime? LastLoginDate { get => _lastLoginDate; set => SetPropertyValueAndDetectChanges(value, ref _lastLoginDate, nameof(LastLoginDate)); } [IgnoreDataMember] - public DateTime LastPasswordChangeDate + public DateTime? LastPasswordChangeDate { get => _lastPasswordChangedDate; set => SetPropertyValueAndDetectChanges(value, ref _lastPasswordChangedDate, nameof(LastPasswordChangeDate)); } [IgnoreDataMember] - public DateTime LastLockoutDate + public DateTime? LastLockoutDate { get => _lastLockoutDate; set => SetPropertyValueAndDetectChanges(value, ref _lastLockoutDate, nameof(LastLockoutDate)); diff --git a/src/Umbraco.Core/Persistence/Repositories/IMemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IMemberRepository.cs index 9b24d60a9f..529680de0a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IMemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IMemberRepository.cs @@ -51,6 +51,7 @@ namespace Umbraco.Cms.Core.Persistence.Repositories /// updating their login date. This operation must be fast and cannot use database locks which is fine if we are only executing a single query /// for this data since there won't be any other data contention issues. /// + [Obsolete("This is now a NoOp since last login date is no longer an umbraco property, set the date on the IMember directly and Save it instead, scheduled for removal in V11.")] void SetLastLogin(string username, DateTime date); } } diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 0b8d981143..70d2bc9996 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -67,10 +67,10 @@ namespace Umbraco.Cms.Core.Services query = Query(); break; case MemberCountType.LockedOut: - query = Query().Where(x => x.PropertyTypeAlias == Constants.Conventions.Member.IsLockedOut && ((Member) x).BoolPropertyValue); + query = Query().Where(x => x.IsLockedOut == true); break; case MemberCountType.Approved: - query = Query().Where(x => x.PropertyTypeAlias == Constants.Conventions.Member.IsApproved && ((Member) x).BoolPropertyValue); + query = Query().Where(x => x.IsApproved == true); break; default: throw new ArgumentOutOfRangeException(nameof(countType)); @@ -748,13 +748,9 @@ namespace Umbraco.Cms.Core.Services #region Save /// + [Obsolete("This is now a NoOp since last login date is no longer an umbraco property, set the date on the IMember directly and Save it instead, scheduled for removal in V11.")] public void SetLastLogin(string username, DateTime date) { - using (var scope = ScopeProvider.CreateScope()) - { - _memberRepository.SetLastLogin(username, date); - scope.Complete(); - } } /// diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs index 43d1a30c2f..f8a88325e9 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs @@ -638,13 +638,15 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install // Membership property types. if (_database.Exists(11)) { - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 28, UniqueId = 28.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.Textarea, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.Comments, Name = Cms.Core.Constants.Conventions.Member.CommentsLabel, SortOrder = 0, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 29, UniqueId = 29.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.LabelInt, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.FailedPasswordAttempts, Name = Cms.Core.Constants.Conventions.Member.FailedPasswordAttemptsLabel, SortOrder = 1, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 30, UniqueId = 30.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.Boolean, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.IsApproved, Name = Cms.Core.Constants.Conventions.Member.IsApprovedLabel, SortOrder = 2, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 31, UniqueId = 31.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.Boolean, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.IsLockedOut, Name = Cms.Core.Constants.Conventions.Member.IsLockedOutLabel, SortOrder = 3, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 32, UniqueId = 32.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.LastLockoutDate, Name = Cms.Core.Constants.Conventions.Member.LastLockoutDateLabel, SortOrder = 4, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 33, UniqueId = 33.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.LastLoginDate, Name = Cms.Core.Constants.Conventions.Member.LastLoginDateLabel, SortOrder = 5, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); - _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 34, UniqueId = 34.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Cms.Core.Constants.Conventions.Member.LastPasswordChangeDate, Name = Cms.Core.Constants.Conventions.Member.LastPasswordChangeDateLabel, SortOrder = 6, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); + _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.PropertyType, "id", false, + new PropertyTypeDto + { + Id = 28, UniqueId = 28.ToGuid(), DataTypeId = Cms.Core.Constants.DataTypes.Textarea, + ContentTypeId = 1044, PropertyTypeGroupId = 11, + Alias = Cms.Core.Constants.Conventions.Member.Comments, + Name = Cms.Core.Constants.Conventions.Member.CommentsLabel, SortOrder = 0, Mandatory = false, + ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing + }); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index d3a920a60b..8210f76cf1 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Semver; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.Common; +using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_10_0_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_0_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_0_1; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_1_0; @@ -284,6 +285,9 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade // TO 9.4.0 To("{DBBA1EA0-25A1-4863-90FB-5D306FB6F1E1}"); To("{DED98755-4059-41BB-ADBD-3FEAB12D1D7B}"); + + // TO 10.0.0 + To("{B7E0D53C-2B0E-418B-AB07-2DDE486E225F}"); } } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_10_0_0/AddMemberPropertiesAsColumns.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_10_0_0/AddMemberPropertiesAsColumns.cs new file mode 100644 index 0000000000..5bc58c9b25 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_10_0_0/AddMemberPropertiesAsColumns.cs @@ -0,0 +1,242 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text; +using NPoco; +using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_10_0_0; + +public class AddMemberPropertiesAsColumns : MigrationBase +{ + public AddMemberPropertiesAsColumns(IMigrationContext context) + : base(context) + { + } + + protected override void Migrate() + { + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); + + AddColumnIfNotExists(columns, "failedPasswordAttempts"); + AddColumnIfNotExists(columns, "isLockedOut"); + AddColumnIfNotExists(columns, "isApproved"); + AddColumnIfNotExists(columns, "lastLoginDate"); + AddColumnIfNotExists(columns, "lastLockoutDate"); + AddColumnIfNotExists(columns, "lastPasswordChangeDate"); + + Sql newestContentVersionQuery = Database.SqlContext.Sql() + .Select($"MAX({GetQuotedSelector("cv", "id")}) as {SqlSyntax.GetQuotedColumnName("id")}", GetQuotedSelector("cv", "nodeId")) + .From("cv") + .GroupBy(GetQuotedSelector("cv", "nodeId")); + + Sql passwordAttemptsQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberFailedPasswordAttempts'"); + + Sql memberApprovedQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberApproved'"); + + Sql memberLockedOutQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberLockedOut'"); + + Sql memberLastLockoutDateQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberLastLockoutDate'"); + + Sql memberLastLoginDateQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberLastLogin'"); + + Sql memberLastPasswordChangeDateQuery = Database.SqlContext.Sql() + .Select(GetSubQueryColumns()) + .From("pt") + .Where($"{GetQuotedSelector("pt", "Alias")} = 'umbracoMemberLastPasswordChangeDate'"); + + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.AppendLine($"UPDATE {Constants.DatabaseSchema.Tables.Member}"); + queryBuilder.AppendLine("SET"); + queryBuilder.AppendLine($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.FailedPasswordAttempts)} = {GetQuotedSelector("umbracoPropertyData", "intValue")},"); + queryBuilder.AppendLine($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.IsApproved)} = {GetQuotedSelector("pdmp", "intValue")},"); + queryBuilder.AppendLine($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.IsLockedOut)} = {GetQuotedSelector("pdlo", "intValue")},"); + queryBuilder.AppendLine($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.LastLockoutDate)} = {GetQuotedSelector("pdlout", "dateValue")},"); + queryBuilder.AppendLine($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.LastLoginDate)} = {GetQuotedSelector("pdlin", "dateValue")},"); + queryBuilder.Append($"\t{Database.SqlContext.SqlSyntax.GetFieldNameForUpdate(x => x.LastPasswordChangeDate)} = {GetQuotedSelector("pdlpc", "dateValue")}"); + + Sql updateMemberColumnsQuery = Database.SqlContext.Sql(queryBuilder.ToString()) + .From() + .InnerJoin() + .On((left, right) => left.NodeId == right.NodeId) + .InnerJoin() + .On((left, right) => left.ContentTypeId == right.NodeId) + .InnerJoin(newestContentVersionQuery, "umbracoContentVersion") + .On((left, right) => left.NodeId == right.NodeId) + .InnerJoin("m") + .On((left, right) => left.NodeId == right.NodeId, null, "m") + .LeftJoin(passwordAttemptsQuery, "failedAttemptsType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin() + .On((left, right) => left.DataTypeId == right.NodeId) + .LeftJoin() + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id) + .LeftJoin(memberApprovedQuery, "memberApprovedType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin("dtmp") + .On((left, right) => left.DataTypeId == right.NodeId, null, "dtmp") + .LeftJoin("pdmp") + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id, "pdmp") + .LeftJoin(memberLockedOutQuery, "memberLockedOutType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin("dtlo") + .On((left, right) => left.DataTypeId == right.NodeId, null, "dtlo") + .LeftJoin("pdlo") + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id, "pdlo") + .LeftJoin(memberLastLockoutDateQuery, "lastLockOutDateType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin("dtlout") + .On((left, right) => left.DataTypeId == right.NodeId, null, "dtlout") + .LeftJoin("pdlout") + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id, "pdlout") + .LeftJoin(memberLastLoginDateQuery, "lastLoginDateType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin("dtlin") + .On((left, right) => left.DataTypeId == right.NodeId, null, "dtlin") + .LeftJoin("pdlin") + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id, "pdlin") + .LeftJoin(memberLastPasswordChangeDateQuery, "lastPasswordChangeType") + .On((left, right) => left.ContentTypeId == right.ContentTypeId) + .LeftJoin("dtlpc") + .On((left, right) => left.DataTypeId == right.NodeId, null, "dtlpc") + .LeftJoin("pdlpc") + .On((left, middle, right) => left.PropertyTypeId == middle.Id && left.VersionId == right.Id, "pdlpc") + .Where(x => x.NodeObjectType == Constants.ObjectTypes.Member); + + Database.Execute(updateMemberColumnsQuery); + + // Removing old property types and values, since these are no longer needed + // Hard coding the aliases, since we want to be able to delete the constants... + string[] propertyTypesToDelete = + { + "umbracoMemberFailedPasswordAttempts", + "umbracoMemberApproved", + "umbracoMemberLockedOut", + "umbracoMemberLastLockoutDate", + "umbracoMemberLastLogin", + "umbracoMemberLastPasswordChangeDate" + }; + + Sql idQuery = Database.SqlContext.Sql().Select(x => x.Id) + .From() + .WhereIn(x => x.Alias, propertyTypesToDelete); + List idsToDelete = Database.Fetch(idQuery); + + // Firstly deleting the property data due to FK constraints + Sql propertyDataDelete = Database.SqlContext.Sql() + .Delete() + .WhereIn(x => x.PropertyTypeId, idsToDelete); + Database.Execute(propertyDataDelete); + + // Then we can remove the property + Sql propertyTypeDelete = Database.SqlContext.Sql() + .Delete() + .WhereIn(x => x.Id, idsToDelete); + Database.Execute(propertyTypeDelete); + } + + private string GetQuotedSelector(string tableName, string columnName) + => $"{SqlSyntax.GetQuotedTableName(tableName)}.{SqlSyntax.GetQuotedColumnName(columnName)}"; + + private object[] GetSubQueryColumns() => new object[] + { + SqlSyntax.GetQuotedColumnName("contentTypeId"), + SqlSyntax.GetQuotedColumnName("dataTypeId"), + SqlSyntax.GetQuotedColumnName("id"), + }; + + [TableName("failedAttemptsType")] + private class FailedAttempts + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } + + [TableName("memberApprovedType")] + private class MemberApproved + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } + + [TableName("memberLockedOutType")] + private class MemberLockedOut + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } + + [TableName("lastLockOutDateType")] + private class LastLockoutDate + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } + + [TableName("lastLoginDateType")] + private class LastLoginDate + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } + + [TableName("lastPasswordChangeType")] + private class LastPasswordChange + { + [Column("contentTypeId")] + public int ContentTypeId { get; set; } + + [Column("dataTypeId")] + public int DataTypeId { get; set; } + + [Column("id")] + public int Id { get; set; } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs index be34a473c1..ef7e75ccaf 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs @@ -49,7 +49,30 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Dtos [NullSetting(NullSetting = NullSettings.Null)] public DateTime? EmailConfirmedDate { get; set; } - // TODO: It would be SOOOOO much better to store all core member data here instead of hiding it in Umbraco properties + [Column("failedPasswordAttempts")] + [NullSetting(NullSetting = NullSettings.Null)] + public int? FailedPasswordAttempts { get; set; } + + [Column("isLockedOut")] + [Constraint(Default = 0)] + [NullSetting(NullSetting = NullSettings.Null)] + public bool IsLockedOut { get; set; } + + [Column("isApproved")] + [Constraint(Default = 1)] + public bool IsApproved { get; set; } + + [Column("lastLoginDate")] + [NullSetting(NullSetting = NullSettings.Null)] + public DateTime? LastLoginDate { get; set; } + + [Column("lastLockoutDate")] + [NullSetting(NullSetting = NullSettings.Null)] + public DateTime? LastLockoutDate { get; set; } + + [Column("lastPasswordChangeDate")] + [NullSetting(NullSetting = NullSettings.Null)] + public DateTime? LastPasswordChangeDate { get; set; } [ResultColumn] [Reference(ReferenceType.OneToOne, ReferenceMemberName = "NodeId")] diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs index a451ac0879..f3c75bda09 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs @@ -130,7 +130,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories content.PasswordConfiguration = dto.PasswordConfig; content.Key = nodeDto.UniqueId; content.VersionId = contentVersionDto.Id; - + // TODO: missing names? content.Path = nodeDto.Path; @@ -143,6 +143,12 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories content.WriterId = contentVersionDto.UserId ?? Cms.Core.Constants.Security.UnknownUserId; content.CreateDate = nodeDto.CreateDate; content.UpdateDate = contentVersionDto.VersionDate; + content.FailedPasswordAttempts = dto.FailedPasswordAttempts ?? default; + content.IsLockedOut = dto.IsLockedOut; + content.IsApproved = dto.IsApproved; + content.LastLoginDate = dto.LastLoginDate; + content.LastLockoutDate = dto.LastLockoutDate; + content.LastPasswordChangeDate = dto.LastPasswordChangeDate; // reset dirty initial properties (U4-1946) content.ResetDirtyProperties(false); @@ -219,7 +225,13 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories EmailConfirmedDate = entity.EmailConfirmedDate, ContentDto = contentDto, ContentVersionDto = BuildContentVersionDto(entity, contentDto), - PasswordConfig = entity.PasswordConfiguration + PasswordConfig = entity.PasswordConfiguration, + FailedPasswordAttempts = entity.FailedPasswordAttempts, + IsApproved = entity.IsApproved, + IsLockedOut = entity.IsLockedOut, + LastLockoutDate = entity.LastLockoutDate, + LastLoginDate = entity.LastLoginDate, + LastPasswordChangeDate = entity.LastPasswordChangeDate, }; return dto; } diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs index 87124a2d0d..affc4d88c4 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs @@ -28,9 +28,9 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories user.Language = dto.UserLanguage; user.SecurityStamp = dto.SecurityStampToken; user.FailedPasswordAttempts = dto.FailedLoginAttempts ?? 0; - user.LastLockoutDate = dto.LastLockoutDate ?? DateTime.MinValue; - user.LastLoginDate = dto.LastLoginDate ?? DateTime.MinValue; - user.LastPasswordChangeDate = dto.LastPasswordChangeDate ?? DateTime.MinValue; + user.LastLockoutDate = dto.LastLockoutDate; + user.LastLoginDate = dto.LastLoginDate; + user.LastPasswordChangeDate = dto.LastPasswordChangeDate; user.CreateDate = dto.CreateDate; user.UpdateDate = dto.UpdateDate; user.Avatar = dto.Avatar; diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/MemberMapper.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/MemberMapper.cs index 066f1584cd..c9fce21a73 100644 --- a/src/Umbraco.Infrastructure/Persistence/Mappers/MemberMapper.cs +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/MemberMapper.cs @@ -35,14 +35,14 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Mappers DefineMap(nameof(Member.Email), nameof(MemberDto.Email)); DefineMap(nameof(Member.Username), nameof(MemberDto.LoginName)); DefineMap(nameof(Member.RawPasswordValue), nameof(MemberDto.Password)); + DefineMap(nameof(Member.IsApproved), nameof(MemberDto.IsApproved)); + DefineMap(nameof(Member.IsLockedOut), nameof(MemberDto.IsLockedOut)); + DefineMap(nameof(Member.FailedPasswordAttempts), nameof(MemberDto.FailedPasswordAttempts)); + DefineMap(nameof(Member.LastLockoutDate), nameof(MemberDto.LastLockoutDate)); + DefineMap(nameof(Member.LastLoginDate), nameof(MemberDto.LastLoginDate)); + DefineMap(nameof(Member.LastPasswordChangeDate), nameof(MemberDto.LastPasswordChangeDate)); - DefineMap(nameof(Member.IsApproved), nameof(PropertyDataDto.IntegerValue)); - DefineMap(nameof(Member.IsLockedOut), nameof(PropertyDataDto.IntegerValue)); DefineMap(nameof(Member.Comments), nameof(PropertyDataDto.TextValue)); - DefineMap(nameof(Member.FailedPasswordAttempts), nameof(PropertyDataDto.IntegerValue)); - DefineMap(nameof(Member.LastLockoutDate), nameof(PropertyDataDto.DateValue)); - DefineMap(nameof(Member.LastLoginDate), nameof(PropertyDataDto.DateValue)); - DefineMap(nameof(Member.LastPasswordChangeDate), nameof(PropertyDataDto.DateValue)); /* Internal experiment */ DefineMap(nameof(Member.DateTimePropertyValue), nameof(PropertyDataDto.DateValue)); diff --git a/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs index 127cbf9e91..7e783a45cd 100644 --- a/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs @@ -405,6 +405,24 @@ namespace Umbraco.Extensions return sql.InnerJoin(join); } + /// + /// Appends an INNER JOIN clause using a nested query. + /// + /// The SQL statement. + /// The nested sql query. + /// An optional alias for the joined table. + /// A SqlJoin statement. + public static Sql.SqlJoinClause InnerJoin(this Sql sql, Sql nestedSelect, string alias = null) + { + var join = $"({nestedSelect.SQL})"; + if (alias is not null) + { + join += " " + sql.SqlContext.SqlSyntax.GetQuotedTableName(alias); + } + + return sql.InnerJoin(join); + } + /// /// Appends a LEFT JOIN clause to the Sql statement. /// @@ -437,6 +455,24 @@ namespace Umbraco.Extensions string alias = null) => sql.SqlContext.SqlSyntax.LeftJoinWithNestedJoin(sql, nestedJoin, alias); + /// + /// Appends an LEFT JOIN clause using a nested query. + /// + /// The SQL statement. + /// The nested sql query. + /// An optional alias for the joined table. + /// A SqlJoin statement. + public static Sql.SqlJoinClause LeftJoin(this Sql sql, Sql nestedSelect, string alias = null) + { + var join = $"({nestedSelect.SQL})"; + if (alias is not null) + { + join += " " + sql.SqlContext.SqlSyntax.GetQuotedTableName(alias); + } + + return sql.LeftJoin(join); + } + /// /// Appends a RIGHT JOIN clause to the Sql statement. /// diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs index 4a3ed0aba6..6ec353d8df 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs @@ -207,58 +207,11 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } /// + [Obsolete( + "This is now a NoOp since last login date is no longer an umbraco property, set the date on the IMember directly and Save it instead, scheduled for removal in V11.")] public void SetLastLogin(string username, DateTime date) { - // Important - these queries are designed to execute without an exclusive WriteLock taken in our distributed lock - // table. However due to the data that we are updating which relies on version data we cannot update this data - // without taking some locks, otherwise we'll end up with strange situations because when a member is updated, that operation - // deletes and re-inserts all property data. So if there are concurrent transactions, one deleting and re-inserting and another trying - // to update there can be problems. This is only an issue for cmsPropertyData, not umbracoContentVersion because that table just - // maintains a single row and it isn't deleted/re-inserted. - // So the important part here is the ForUpdate() call on the select to fetch the property data to update. - // Update the cms property value for the member - - SqlTemplate sqlSelectTemplateProperty = SqlContext.Templates.Get( - "Umbraco.Core.MemberRepository.SetLastLogin1", s => s - .Select(x => x.Id) - .From() - .InnerJoin() - .On((l, r) => l.Id == r.PropertyTypeId) - .InnerJoin() - .On((l, r) => l.Id == r.VersionId) - .InnerJoin().On((l, r) => l.NodeId == r.NodeId) - .InnerJoin().On((l, r) => l.NodeId == r.NodeId) - .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType")) - .Where(x => x.Alias == SqlTemplate.Arg("propertyTypeAlias")) - .Where(x => x.LoginName == SqlTemplate.Arg("username")) - .ForUpdate()); - Sql sqlSelectProperty = sqlSelectTemplateProperty.Sql(Constants.ObjectTypes.Member, - Constants.Conventions.Member.LastLoginDate, username); - - Sql update = Sql() - .Update(u => u - .Set(x => x.DateValue, date)) - .WhereIn(x => x.Id, sqlSelectProperty); - - Database.Execute(update); - - // Update the umbracoContentVersion value for the member - - SqlTemplate sqlSelectTemplateVersion = SqlContext.Templates.Get( - "Umbraco.Core.MemberRepository.SetLastLogin2", s => s - .Select(x => x.Id) - .From() - .InnerJoin().On((l, r) => l.NodeId == r.NodeId) - .InnerJoin().On((l, r) => l.NodeId == r.NodeId) - .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType")) - .Where(x => x.LoginName == SqlTemplate.Arg("username"))); - Sql sqlSelectVersion = sqlSelectTemplateVersion.Sql(Constants.ObjectTypes.Member, username); - - Database.Execute(Sql() - .Update(u => u - .Set(x => x.VersionDate, date)) - .WhereIn(x => x.Id, sqlSelectVersion)); } /// @@ -772,12 +725,43 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement changedCols.Add("LoginName"); } + if (entity.IsPropertyDirty(nameof(entity.FailedPasswordAttempts))) + { + changedCols.Add(nameof(entity.FailedPasswordAttempts)); + } + + if (entity.IsPropertyDirty(nameof(entity.IsApproved))) + { + changedCols.Add(nameof(entity.IsApproved)); + } + + if (entity.IsPropertyDirty(nameof(entity.IsLockedOut))) + { + changedCols.Add(nameof(entity.IsLockedOut)); + } + + if (entity.IsPropertyDirty(nameof(entity.LastLockoutDate))) + { + changedCols.Add(nameof(entity.LastLockoutDate)); + } + + if (entity.IsPropertyDirty(nameof(entity.LastLoginDate))) + { + changedCols.Add(nameof(entity.LastLoginDate)); + } + + if (entity.IsPropertyDirty(nameof(entity.LastPasswordChangeDate))) + { + changedCols.Add(nameof(entity.LastPasswordChangeDate)); + } + // this can occur from an upgrade if (memberDto.PasswordConfig.IsNullOrWhiteSpace()) { memberDto.PasswordConfig = DefaultPasswordConfigJson; changedCols.Add("passwordConfig"); - }else if (memberDto.PasswordConfig == Constants.Security.UnknownPasswordConfigJson) + } + else if (memberDto.PasswordConfig == Constants.Security.UnknownPasswordConfigJson) { changedCols.Add("passwordConfig"); } diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs index 7e3c474cef..4bd8e1cece 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs @@ -496,12 +496,12 @@ namespace Umbraco.Cms.Core.Security // don't assign anything if nothing has changed as this will trigger the track changes of the model if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastLoginDateUtc)) || (user.LastLoginDate != default && identityUser.LastLoginDateUtc.HasValue == false) - || (identityUser.LastLoginDateUtc.HasValue && user.LastLoginDate.ToUniversalTime() != identityUser.LastLoginDateUtc.Value)) + || (identityUser.LastLoginDateUtc.HasValue && user.LastLoginDate?.ToUniversalTime() != identityUser.LastLoginDateUtc.Value)) { anythingChanged = true; // if the LastLoginDate is being set to MinValue, don't convert it ToLocalTime - DateTime dt = identityUser.LastLoginDateUtc == DateTime.MinValue ? DateTime.MinValue : identityUser.LastLoginDateUtc.Value.ToLocalTime(); + DateTime? dt = identityUser.LastLoginDateUtc?.ToLocalTime(); user.LastLoginDate = dt; } @@ -513,11 +513,11 @@ namespace Umbraco.Cms.Core.Security } if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastPasswordChangeDateUtc)) - || (user.LastPasswordChangeDate != default && identityUser.LastPasswordChangeDateUtc.HasValue == false) - || (identityUser.LastPasswordChangeDateUtc.HasValue && user.LastPasswordChangeDate.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value)) + || (user.LastPasswordChangeDate.HasValue && user.LastPasswordChangeDate.Value != default && identityUser.LastPasswordChangeDateUtc.HasValue == false) + || (identityUser.LastPasswordChangeDateUtc.HasValue && user.LastPasswordChangeDate?.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value)) { anythingChanged = true; - user.LastPasswordChangeDate = identityUser.LastPasswordChangeDateUtc.Value.ToLocalTime(); + user.LastPasswordChangeDate = identityUser.LastPasswordChangeDateUtc?.ToLocalTime(); } if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.EmailConfirmed)) diff --git a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs index 3b51233f61..9d8c29bb6d 100644 --- a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs +++ b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs @@ -72,8 +72,8 @@ namespace Umbraco.Cms.Core.Security target.CalculatedContentStartNodeIds = source.CalculateContentStartNodeIds(_entityService, _appCaches); target.Email = source.Email; target.UserName = source.Username; - target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate.ToUniversalTime(); - target.LastLoginDateUtc = source.LastLoginDate.ToUniversalTime(); + target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate?.ToUniversalTime(); + target.LastLoginDateUtc = source.LastLoginDate?.ToUniversalTime(); target.InviteDateUtc = source.InvitedDate?.ToUniversalTime(); target.EmailConfirmed = source.EmailConfirmedDate.HasValue; target.Name = source.Name; @@ -93,8 +93,8 @@ namespace Umbraco.Cms.Core.Security { target.Email = source.Email; target.UserName = source.Username; - target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate.ToUniversalTime(); - target.LastLoginDateUtc = source.LastLoginDate.ToUniversalTime(); + target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate?.ToUniversalTime(); + target.LastLoginDateUtc = source.LastLoginDate?.ToUniversalTime(); target.EmailConfirmed = source.EmailConfirmedDate.HasValue; target.Name = source.Name; target.AccessFailedCount = source.FailedPasswordAttempts; @@ -104,7 +104,7 @@ namespace Umbraco.Cms.Core.Security target.SecurityStamp = source.SecurityStamp; target.LockoutEnd = source.IsLockedOut ? DateTime.MaxValue.ToUniversalTime() : (DateTime?)null; target.Comments = source.Comments; - target.LastLockoutDateUtc = source.LastLockoutDate == DateTime.MinValue ? null : source.LastLockoutDate.ToUniversalTime(); + target.LastLockoutDateUtc = source.LastLockoutDate == DateTime.MinValue ? null : source.LastLockoutDate?.ToUniversalTime(); target.CreatedDateUtc = source.CreateDate.ToUniversalTime(); target.Key = source.Key; target.MemberTypeAlias = source.ContentTypeAlias; diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index d2b0db111c..8fe802272a 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -174,6 +174,7 @@ namespace Umbraco.Cms.Core.Security //TODO: should this be thrown, or an identity result? throw new InvalidOperationException("The user id must be an integer to work with the Umbraco"); } + using IScope scope = _scopeProvider.CreateScope(autoComplete: true); IMember found = _memberService.GetById(asInt); @@ -183,17 +184,10 @@ namespace Umbraco.Cms.Core.Security var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MemberIdentityUser.Logins)); var isTokensPropertyDirty = user.IsPropertyDirty(nameof(MemberIdentityUser.LoginTokens)); - MemberDataChangeType memberChangeType = UpdateMemberProperties(found, user); - if (memberChangeType == MemberDataChangeType.FullSave) + if (UpdateMemberProperties(found, user)) { _memberService.Save(found); } - else if (memberChangeType == MemberDataChangeType.LoginOnly) - { - // If the member is only logging in, just issue that command without - // any write locks so we are creating a bottleneck. - _memberService.SetLastLogin(found.Username, DateTime.Now); - } if (isLoginsPropertyDirty) { @@ -588,19 +582,16 @@ namespace Umbraco.Cms.Core.Security return user; } - private MemberDataChangeType UpdateMemberProperties(IMember member, MemberIdentityUser identityUser) + private bool UpdateMemberProperties(IMember member, MemberIdentityUser identityUser) { - MemberDataChangeType changeType = MemberDataChangeType.None; + var anythingChanged = false; // don't assign anything if nothing has changed as this will trigger the track changes of the model if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.LastLoginDateUtc)) || (member.LastLoginDate != default && identityUser.LastLoginDateUtc.HasValue == false) - || (identityUser.LastLoginDateUtc.HasValue && member.LastLoginDate.ToUniversalTime() != identityUser.LastLoginDateUtc.Value)) + || (identityUser.LastLoginDateUtc.HasValue && member.LastLoginDate?.ToUniversalTime() != identityUser.LastLoginDateUtc.Value)) { - // If the LastLoginDate is default on the member we have to do a full save. - // This is because the umbraco property data for the member doesn't exist yet in this case - // meaning we can't just update that property data, but have to do a full save to create it - changeType = member.LastLoginDate == default ? MemberDataChangeType.FullSave : MemberDataChangeType.LoginOnly; + anythingChanged = true; // if the LastLoginDate is being set to MinValue, don't convert it ToLocalTime DateTime dt = identityUser.LastLoginDateUtc == DateTime.MinValue ? DateTime.MinValue : identityUser.LastLoginDateUtc.Value.ToLocalTime(); @@ -609,16 +600,16 @@ namespace Umbraco.Cms.Core.Security if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.LastPasswordChangeDateUtc)) || (member.LastPasswordChangeDate != default && identityUser.LastPasswordChangeDateUtc.HasValue == false) - || (identityUser.LastPasswordChangeDateUtc.HasValue && member.LastPasswordChangeDate.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value)) + || (identityUser.LastPasswordChangeDateUtc.HasValue && member.LastPasswordChangeDate?.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value)) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.LastPasswordChangeDate = identityUser.LastPasswordChangeDateUtc.Value.ToLocalTime(); } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.Comments)) && member.Comments != identityUser.Comments && identityUser.Comments.IsNullOrWhiteSpace() == false) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.Comments = identityUser.Comments; } @@ -626,34 +617,34 @@ namespace Umbraco.Cms.Core.Security || (member.EmailConfirmedDate.HasValue && member.EmailConfirmedDate.Value != default && identityUser.EmailConfirmed == false) || ((member.EmailConfirmedDate.HasValue == false || member.EmailConfirmedDate.Value == default) && identityUser.EmailConfirmed)) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.EmailConfirmedDate = identityUser.EmailConfirmed ? (DateTime?)DateTime.Now : null; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.Name)) && member.Name != identityUser.Name && identityUser.Name.IsNullOrWhiteSpace() == false) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.Name = identityUser.Name; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.Email)) && member.Email != identityUser.Email && identityUser.Email.IsNullOrWhiteSpace() == false) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.Email = identityUser.Email; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.AccessFailedCount)) && member.FailedPasswordAttempts != identityUser.AccessFailedCount) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.FailedPasswordAttempts = identityUser.AccessFailedCount; } if (member.IsLockedOut != identityUser.IsLockedOut) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.IsLockedOut = identityUser.IsLockedOut; if (member.IsLockedOut) @@ -665,40 +656,40 @@ namespace Umbraco.Cms.Core.Security if (member.IsApproved != identityUser.IsApproved) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.IsApproved = identityUser.IsApproved; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.UserName)) && member.Username != identityUser.UserName && identityUser.UserName.IsNullOrWhiteSpace() == false) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.Username = identityUser.UserName; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.PasswordHash)) && member.RawPasswordValue != identityUser.PasswordHash && identityUser.PasswordHash.IsNullOrWhiteSpace() == false) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.RawPasswordValue = identityUser.PasswordHash; member.PasswordConfiguration = identityUser.PasswordConfig; } if (member.PasswordConfiguration != identityUser.PasswordConfig) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.PasswordConfiguration = identityUser.PasswordConfig; } if (member.SecurityStamp != identityUser.SecurityStamp) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; member.SecurityStamp = identityUser.SecurityStamp; } if (identityUser.IsPropertyDirty(nameof(MemberIdentityUser.Roles))) { - changeType = MemberDataChangeType.FullSave; + anythingChanged = true; var identityUserRoles = identityUser.Roles.Select(x => x.RoleId).ToArray(); _memberService.ReplaceRoles(new[] { member.Id }, identityUserRoles); @@ -707,7 +698,7 @@ namespace Umbraco.Cms.Core.Security // reset all changes identityUser.ResetDirtyProperties(false); - return changeType; + return anythingChanged; } public IPublishedContent GetPublishedMember(MemberIdentityUser user) @@ -725,13 +716,6 @@ namespace Umbraco.Cms.Core.Security return publishedSnapshot.Members.Get(member); } - private enum MemberDataChangeType - { - None, - LoginOnly, - FullSave - } - /// /// Overridden to support Umbraco's own data storage requirements /// diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs b/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs index 53cc597cf5..69a3230187 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs @@ -109,13 +109,13 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache public bool IsLockedOut => Member.IsLockedOut; - public DateTime LastLockoutDate => Member.LastLockoutDate; + public DateTime? LastLockoutDate => Member.LastLockoutDate; public DateTime CreationDate => Member.CreateDate; - public DateTime LastLoginDate => Member.LastLoginDate; + public DateTime? LastLoginDate => Member.LastLoginDate; - public DateTime LastPasswordChangedDate => Member.LastPasswordChangeDate; + public DateTime? LastPasswordChangedDate => Member.LastPasswordChangeDate; #endregion } diff --git a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs index 5ab88aa63b..68903a0c79 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs @@ -847,7 +847,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers // Check user hasn't logged in. If they have they may have made content changes which will mean // the Id is associated with audit trails, versions etc. and can't be removed. - if (user.LastLoginDate != default(DateTime)) + if (user.LastLoginDate is not null && user.LastLoginDate != default(DateTime)) { return BadRequest(); } diff --git a/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs b/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs index 8da173ce68..84862c4088 100644 --- a/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs @@ -64,6 +64,8 @@ namespace Umbraco.Cms.Web.BackOffice.Mapping //Membership target.Username = source.Username; target.Email = source.Email; + target.IsLockedOut = source.IsLockedOut; + target.IsApproved = source.IsApproved; target.MembershipProperties = _tabsAndPropertiesMapper.MapMembershipProperties(source, context); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index 38b56b7661..805e222920 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -290,6 +290,12 @@ case '_umb_membergroup': saveModel.memberGroups = _.keys(_.pick(prop.value, value => value === true)); break; + case '_umb_approved': + saveModel.isApproved = prop.value; + break; + case '_umb_lockedOut': + saveModel.isLockedOut = prop.value; + break; } }); diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml index 9916973405..676c502e15 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml @@ -1916,6 +1916,7 @@ Mange hilsner fra Umbraco robotten Aktiv Deaktiveret Låst ude + Godkendt Inviteret Inaktiv Navn (A-Å) diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml index 6670f692b8..2e7711bc88 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml @@ -1520,7 +1520,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont Insufficient user permissions, could not complete the operation Cancelled Operation was cancelled by a 3rd party add-in - This file is being uploaded as part of a folder, but creating a new folder is not allowed here + This file is being uploaded as part of a folder, but creating a new folder is not allowed here Creating a new folder is not allowed here Publishing was cancelled by a 3rd party add-in Property type already exists @@ -2217,6 +2217,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont Active Disabled Locked out + Approved Invited Inactive Name (A-Z) diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml index bc8f38d408..5850e54b6e 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml @@ -2293,6 +2293,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont Active Disabled Locked out + Approved Invited Inactive Name (A-Z) diff --git a/tests/Umbraco.Tests.Common/Builders/MemberTypeBuilder.cs b/tests/Umbraco.Tests.Common/Builders/MemberTypeBuilder.cs index fd8e687c34..9cf092d6d4 100644 --- a/tests/Umbraco.Tests.Common/Builders/MemberTypeBuilder.cs +++ b/tests/Umbraco.Tests.Common/Builders/MemberTypeBuilder.cs @@ -35,47 +35,11 @@ namespace Umbraco.Cms.Tests.Common.Builders .WithId(99) .WithName(Constants.Conventions.Member.StandardPropertiesGroupName) .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextArea) - .WithValueStorageType(ValueStorageType.Ntext) - .WithAlias(Constants.Conventions.Member.Comments) - .WithName(Constants.Conventions.Member.CommentsLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Boolean) - .WithValueStorageType(ValueStorageType.Integer) - .WithAlias(Constants.Conventions.Member.IsApproved) - .WithName(Constants.Conventions.Member.IsApprovedLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Boolean) - .WithValueStorageType(ValueStorageType.Integer) - .WithAlias(Constants.Conventions.Member.IsLockedOut) - .WithName(Constants.Conventions.Member.IsLockedOutLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Label) - .WithValueStorageType(ValueStorageType.Date) - .WithAlias(Constants.Conventions.Member.LastLoginDate) - .WithName(Constants.Conventions.Member.LastLoginDateLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Label) - .WithValueStorageType(ValueStorageType.Date) - .WithAlias(Constants.Conventions.Member.LastPasswordChangeDate) - .WithName(Constants.Conventions.Member.LastPasswordChangeDateLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Label) - .WithValueStorageType(ValueStorageType.Date) - .WithAlias(Constants.Conventions.Member.LastLockoutDate) - .WithName(Constants.Conventions.Member.LastLockoutDateLabel) - .Done() - .AddPropertyType() - .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.Label) - .WithValueStorageType(ValueStorageType.Integer) - .WithAlias(Constants.Conventions.Member.FailedPasswordAttempts) - .WithName(Constants.Conventions.Member.FailedPasswordAttemptsLabel) - .Done(); + .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextArea) + .WithValueStorageType(ValueStorageType.Ntext) + .WithAlias(Constants.Conventions.Member.Comments) + .WithName(Constants.Conventions.Member.CommentsLabel) + .Done(); _propertyGroupBuilders.Add(builder); return this; } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberServiceTests.cs index 2fe543c1e2..c2316b7752 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberServiceTests.cs @@ -83,12 +83,14 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) { LastLoginDate = now, - UpdateDate = now + UpdateDate = now, }; MemberService.Save(member); DateTime newDate = now.AddDays(10); - MemberService.SetLastLogin(member.Username, newDate); + member.LastLoginDate = newDate; + member.UpdateDate = newDate; + MemberService.Save(member); // re-get member = MemberService.GetById(member.Id); @@ -97,6 +99,121 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services Assert.That(member.UpdateDate, Is.EqualTo(newDate).Within(1).Seconds); } + // These might seem like some somewhat pointless tests, but there's some funky things going on in MemberRepository when saving. + [Test] + public void Can_Set_Failed_Password_Attempts() + { + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) + { + FailedPasswordAttempts = 1, + }; + MemberService.Save(member); + + member.FailedPasswordAttempts = 2; + MemberService.Save(member); + + member = MemberService.GetById(member.Id); + + Assert.AreEqual(2, member.FailedPasswordAttempts); + } + + [Test] + public void Can_Set_Is_Approved() + { + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true); + MemberService.Save(member); + + member.IsApproved = false; + MemberService.Save(member); + + member = MemberService.GetById(member.Id); + + Assert.IsFalse(member.IsApproved); + } + + [Test] + public void Can_Set_Is_Locked_Out() + { + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) + { + IsLockedOut = false, + }; + MemberService.Save(member); + + member.IsLockedOut = true; + MemberService.Save(member); + + member = MemberService.GetById(member.Id); + + Assert.IsTrue(member.IsLockedOut); + } + + [Test] + public void Can_Set_Last_Lockout_Date() + { + DateTime now = DateTime.Now; + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) + { + LastLockoutDate = now, + }; + MemberService.Save(member); + + DateTime newDate = now.AddDays(10); + member.LastLockoutDate = newDate; + MemberService.Save(member); + + // re-get + member = MemberService.GetById(member.Id); + + Assert.That(member.LastLockoutDate, Is.EqualTo(newDate).Within(1).Seconds); + } + + [Test] + public void Can_set_Last_Login_Date() + { + DateTime now = DateTime.Now; + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) + { + LastLoginDate = now, + }; + MemberService.Save(member); + + DateTime newDate = now.AddDays(10); + member.LastLoginDate = newDate; + MemberService.Save(member); + + // re-get + member = MemberService.GetById(member.Id); + + Assert.That(member.LastLoginDate, Is.EqualTo(newDate).Within(1).Seconds); + } + + [Test] + public void Can_Set_Last_Password_Change_Date() + { + DateTime now = DateTime.Now; + IMemberType memberType = MemberTypeService.Get("member"); + IMember member = new Member("xname", "xemail", "xusername", "xrawpassword", memberType, true) + { + LastPasswordChangeDate = now, + }; + MemberService.Save(member); + + DateTime newDate = now.AddDays(10); + member.LastPasswordChangeDate = newDate; + MemberService.Save(member); + + // re-get + member = MemberService.GetById(member.Id); + + Assert.That(member.LastPasswordChangeDate, Is.EqualTo(newDate).Within(1).Seconds); + } + [Test] public void Can_Create_Member_With_Properties() { @@ -116,17 +233,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services // contains the umbracoMember... properties created when installing, on the member type // contains the other properties, that PublishedContentType adds (BuiltinMemberProperties) - // - // TODO: see TODO in PublishedContentType, this list contains duplicates string[] aliases = new[] { Constants.Conventions.Member.Comments, - Constants.Conventions.Member.FailedPasswordAttempts, - Constants.Conventions.Member.IsApproved, - Constants.Conventions.Member.IsLockedOut, - Constants.Conventions.Member.LastLockoutDate, - Constants.Conventions.Member.LastLoginDate, - Constants.Conventions.Member.LastPasswordChangeDate, nameof(IMember.Email), nameof(IMember.Username), nameof(IMember.Comments), @@ -589,14 +698,25 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services // just a c# property of the Member object resolved.Email = "changed@test.com"; + // NOTE: This will not trigger a property isDirty for the same reason above, but this is a new change, so leave this to make sure. + resolved.FailedPasswordAttempts = 1234; + // NOTE: this WILL trigger a property isDirty because setting this c# property actually sets a value of // the underlying 'Property' - resolved.FailedPasswordAttempts = 1234; + resolved.Comments = "This will make it dirty"; var dirtyMember = (ICanBeDirty)resolved; var dirtyProperties = resolved.Properties.Where(x => x.IsDirty()).ToList(); Assert.IsTrue(dirtyMember.IsDirty()); Assert.AreEqual(1, dirtyProperties.Count); + + // Assert that email and failed password attempts is still set as dirty on the member it self + Assert.IsTrue(dirtyMember.IsPropertyDirty(nameof(resolved.Email))); + Assert.IsTrue(dirtyMember.IsPropertyDirty(nameof(resolved.FailedPasswordAttempts))); + + // Comment will also be marked as dirty on the member object because content base merges dirty properties. + Assert.IsTrue(dirtyMember.IsPropertyDirty(Constants.Conventions.Member.Comments)); + Assert.AreEqual(3, dirtyMember.GetDirtyProperties().Count()); } [Test] @@ -1205,7 +1325,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services MemberService.Save(members); Member customMember = MemberBuilder.CreateSimpleMember(memberType, "hello", "hello@test.com", "hello", "hello"); - customMember.SetValue(Constants.Conventions.Member.IsLockedOut, true); + customMember.IsLockedOut = true; MemberService.Save(customMember); int found = MemberService.GetCount(MemberCountType.LockedOut); @@ -1222,7 +1342,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services MemberService.Save(members); Member customMember = MemberBuilder.CreateSimpleMember(memberType, "hello", "hello@test.com", "hello", "hello"); - customMember.SetValue(Constants.Conventions.Member.IsApproved, false); + customMember.IsApproved = false; MemberService.Save(customMember); int found = MemberService.GetCount(MemberCountType.Approved); @@ -1250,48 +1370,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services Assert.IsTrue(found.Comments.IsNullOrWhiteSpace()); } - /// - /// Because we are forcing some of the built-ins to be Labels which have an underlying db type as nvarchar but we need - /// to ensure that the dates/int get saved to the correct column anyways. - /// - [Test] - public void Setting_DateTime_Property_On_Built_In_Member_Property_Saves_To_Correct_Column() - { - IMemberType memberType = MemberTypeBuilder.CreateSimpleMemberType(); - MemberTypeService.Save(memberType); - Member member = MemberBuilder.CreateSimpleMember(memberType, "test", "test@test.com", "test", "test"); - DateTime date = DateTime.Now; - member.LastLoginDate = DateTime.Now; - MemberService.Save(member); - - IMember result = MemberService.GetById(member.Id); - Assert.AreEqual( - date.TruncateTo(DateTimeExtensions.DateTruncate.Second), - result.LastLoginDate.TruncateTo(DateTimeExtensions.DateTruncate.Second)); - - // now ensure the col is correct - ISqlContext sqlContext = GetRequiredService(); - Sql sql = sqlContext.Sql().Select() - .From() - .InnerJoin().On(dto => dto.PropertyTypeId, dto => dto.Id) - .InnerJoin().On((left, right) => left.VersionId == right.Id) - .Where(dto => dto.NodeId == member.Id) - .Where(dto => dto.Alias == Constants.Conventions.Member.LastLoginDate); - - List colResult; - using (IScope scope = ScopeProvider.CreateScope()) - { - colResult = ScopeAccessor.AmbientScope.Database.Fetch(sql); - scope.Complete(); - } - - Assert.AreEqual(1, colResult.Count); - Assert.IsTrue(colResult.First().DateValue.HasValue); - Assert.IsFalse(colResult.First().IntegerValue.HasValue); - Assert.IsNull(colResult.First().TextValue); - Assert.IsNull(colResult.First().VarcharValue); - } - [Test] public void New_Member_Approved_By_Default() { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberBuilderTests.cs index f7392f9e38..6cddecc473 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberBuilderTests.cs @@ -137,14 +137,14 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Tests.Common.Builders Assert.AreEqual(testLastLoginDate, member.LastLoginDate); Assert.AreEqual(testLastPasswordChangeDate, member.LastPasswordChangeDate); Assert.AreEqual(testGroups, member.Groups.ToArray()); - Assert.AreEqual(10, member.Properties.Count); // 7 from membership properties group, 3 custom + Assert.AreEqual(4, member.Properties.Count); // 1 from membership properties group, 3 custom Assert.AreEqual(testPropertyData1.Value, member.GetValue(testPropertyData1.Key)); Assert.AreEqual(testPropertyData2.Value, member.GetValue(testPropertyData2.Key)); Assert.AreEqual(testPropertyData3.Value, member.GetValue(testPropertyData3.Key)); IOrderedEnumerable propertyIds = member.Properties.Select(x => x.Id).OrderBy(x => x); Assert.AreEqual(testPropertyIdsIncrementingFrom + 1, propertyIds.Min()); - Assert.AreEqual(testPropertyIdsIncrementingFrom + 10, propertyIds.Max()); + Assert.AreEqual(testPropertyIdsIncrementingFrom + 4, propertyIds.Max()); Assert.AreEqual(2, member.AdditionalData.Count); Assert.AreEqual(testAdditionalData1.Value, member.AdditionalData[testAdditionalData1.Key]); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberTypeBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberTypeBuilderTests.cs index 925b9386a6..239674aaa8 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberTypeBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Tests.Common/Builders/MemberTypeBuilderTests.cs @@ -100,11 +100,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Tests.Common.Builders Assert.AreEqual(testThumbnail, memberType.Thumbnail); Assert.AreEqual(testTrashed, memberType.Trashed); Assert.IsFalse(memberType.IsContainer); - Assert.AreEqual(9, memberType.PropertyTypes.Count()); // 7 from membership properties group, 2 custom + Assert.AreEqual(3, memberType.PropertyTypes.Count()); // 1 from membership properties group, 2 custom IOrderedEnumerable propertyTypeIds = memberType.PropertyTypes.Select(x => x.Id).OrderBy(x => x); Assert.AreEqual(testPropertyTypeIdsIncrementingFrom + 1, propertyTypeIds.Min()); - Assert.AreEqual(testPropertyTypeIdsIncrementingFrom + 9, propertyTypeIds.Max()); + Assert.AreEqual(testPropertyTypeIdsIncrementingFrom + 3, propertyTypeIds.Max()); Assert.IsTrue(memberType.MemberCanEditProperty(testPropertyType1.Alias)); Assert.IsFalse(memberType.MemberCanViewProperty(testPropertyType1.Alias)); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index aaf3445908..41e0434ebc 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -594,7 +594,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Path = member.Path }; - memberDisplay = new MemberDisplay() + memberDisplay = new MemberDisplay { Id = memberId, SortOrder = member.SortOrder, @@ -609,33 +609,57 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers ContentTypeName = member.ContentType.Name, Icon = fakeMemberData.Icon, Path = member.Path, - Tabs = new List>() + Tabs = new List> { - new Tab() + new() { Alias = "test", Id = 77, - Properties = new List() + Properties = new List { - new ContentPropertyDisplay() + new() { - Alias = "_umb_login" + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login", }, - new ContentPropertyDisplay() + new() { - Alias= "_umb_email" + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email", }, - new ContentPropertyDisplay() + new() { - Alias = "_umb_password" + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password", }, - new ContentPropertyDisplay() + new() { - Alias = "_umb_membergroup" - } - } - } - } + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}membergroup", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}failedPasswordAttempts", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}approved", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lockedOut", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLoginDate", + }, + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastPasswordChangeDate", + }, + }, + }, + }, }; return member;