diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml index b04155137b..3e6ca2f8a0 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml @@ -346,6 +346,7 @@ Create a new member All Members Member groups have no additional properties for editing. + Two-Factor Authentication Failed to copy content type diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml b/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml index 4a025ed25c..310111262d 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml @@ -362,6 +362,7 @@ The member already has a password set Lockout is not enabled for this member The member is not in group '%0%' + Two-Factor Authentication Failed to copy content type diff --git a/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs b/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs index 2963618e1b..a610b0f575 100644 --- a/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs +++ b/src/Umbraco.Core/Models/ContentEditing/MemberSave.cs @@ -34,6 +34,9 @@ public class MemberSave : ContentBaseSave [DataMember(Name = "isApproved")] public bool IsApproved { get; set; } + [DataMember(Name = "isTwoFactorEnabled")] + public bool IsTwoFactorEnabled { get; set; } + private T? GetPropertyValue(string alias) { ContentPropertyBasic? prop = Properties.FirstOrDefault(x => x.Alias == alias); diff --git a/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs b/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs index ae9876628f..36d341fb0a 100644 --- a/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs +++ b/src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Dictionary; @@ -6,6 +7,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Core.Models.Mapping; @@ -26,8 +28,36 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper private readonly IMemberService _memberService; private readonly IMemberGroupService _memberGroupService; private readonly MemberPasswordConfigurationSettings _memberPasswordConfiguration; - private readonly PropertyEditorCollection _propertyEditorCollection; + private readonly ITwoFactorLoginService _twoFactorLoginService; + // PropertyEditorCollection is still injected as when removing it, the number of + // parameters matches with the obsolete ctor and the two ctors become ambiguous + // [ActivatorUtilitiesConstructor] won't solve the problem in this case. + // PropertyEditorCollection can be removed when the obsolete ctor is removed for + // Umbraco 13 + public MemberTabsAndPropertiesMapper( + ICultureDictionary cultureDictionary, + IBackOfficeSecurityAccessor backofficeSecurityAccessor, + ILocalizedTextService localizedTextService, + IMemberTypeService memberTypeService, + IMemberService memberService, + IMemberGroupService memberGroupService, + IOptions memberPasswordConfiguration, + IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, + PropertyEditorCollection propertyEditorCollection, + ITwoFactorLoginService twoFactorLoginService) + : base(cultureDictionary, localizedTextService, contentTypeBaseServiceProvider) + { + _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); + _localizedTextService = localizedTextService ?? throw new ArgumentNullException(nameof(localizedTextService)); + _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); + _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); + _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); + _memberPasswordConfiguration = memberPasswordConfiguration.Value; + _twoFactorLoginService = twoFactorLoginService ?? throw new ArgumentNullException(nameof(twoFactorLoginService)); + } + + [Obsolete("Use constructor that also takes an ITwoFactorLoginService. Scheduled for removal in V13")] public MemberTabsAndPropertiesMapper( ICultureDictionary cultureDictionary, IBackOfficeSecurityAccessor backofficeSecurityAccessor, @@ -38,15 +68,18 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper IOptions memberPasswordConfiguration, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, PropertyEditorCollection propertyEditorCollection) - : base(cultureDictionary, localizedTextService, contentTypeBaseServiceProvider) + : this( + cultureDictionary, + backofficeSecurityAccessor, + localizedTextService, + memberTypeService, + memberService, + memberGroupService, + memberPasswordConfiguration, + contentTypeBaseServiceProvider, + propertyEditorCollection, + StaticServiceProvider.Instance.GetRequiredService()) { - _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); - _localizedTextService = localizedTextService ?? throw new ArgumentNullException(nameof(localizedTextService)); - _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); - _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); - _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); - _memberPasswordConfiguration = memberPasswordConfiguration.Value; - _propertyEditorCollection = propertyEditorCollection; } /// @@ -180,6 +213,8 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper public IEnumerable MapMembershipProperties(IMember member, MapperContext? context) { + var isTwoFactorEnabled = _twoFactorLoginService.IsTwoFactorEnabledAsync(member.Key).Result; + var properties = new List { GetLoginProperty(member, _localizedTextService), @@ -245,6 +280,17 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper Readonly = !member.IsLockedOut, // IMember.IsLockedOut can't be set to true, so make it readonly when that's the case (you can only unlock) }, + // Toggle for disabling Two-Factor Authentication for a Member + new() + { + Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}twoFactorEnabled", + Label = _localizedTextService.Localize("member", "2fa"), + Value = isTwoFactorEnabled, + View = "boolean", + IsSensitive = true, + Readonly = !isTwoFactorEnabled, // The value can't be set to true, so make it readonly when that's the case (you can only disable) + }, + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate", diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 70f337f44f..d79d5232ca 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.ContentApps; @@ -25,6 +26,7 @@ using Umbraco.Cms.Web.BackOffice.Filters; using Umbraco.Cms.Web.BackOffice.ModelBinders; using Umbraco.Cms.Web.Common.Attributes; using Umbraco.Cms.Web.Common.Authorization; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Cms.Web.Common.Filters; using Umbraco.Cms.Web.Common.Security; using Umbraco.Extensions; @@ -50,6 +52,7 @@ public class MemberController : ContentControllerBase private readonly IPasswordChanger _passwordChanger; private readonly PropertyEditorCollection _propertyEditors; private readonly ICoreScopeProvider _scopeProvider; + private readonly ITwoFactorLoginService _twoFactorLoginService; private readonly IShortStringHelper _shortStringHelper; private readonly IUmbracoMapper _umbracoMapper; @@ -71,6 +74,43 @@ public class MemberController : ContentControllerBase /// The JSON serializer /// The password changer /// The core scope provider + /// The two factor login service + [ActivatorUtilitiesConstructor] + public MemberController( + ICultureDictionary cultureDictionary, + ILoggerFactory loggerFactory, + IShortStringHelper shortStringHelper, + IEventMessagesFactory eventMessages, + ILocalizedTextService localizedTextService, + PropertyEditorCollection propertyEditors, + IUmbracoMapper umbracoMapper, + IMemberService memberService, + IMemberTypeService memberTypeService, + IMemberManager memberManager, + IDataTypeService dataTypeService, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IJsonSerializer jsonSerializer, + IPasswordChanger passwordChanger, + ICoreScopeProvider scopeProvider, + ITwoFactorLoginService twoFactorLoginService) + : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer) + { + _propertyEditors = propertyEditors; + _umbracoMapper = umbracoMapper; + _memberService = memberService; + _memberTypeService = memberTypeService; + _memberManager = memberManager; + _dataTypeService = dataTypeService; + _localizedTextService = localizedTextService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + _jsonSerializer = jsonSerializer; + _shortStringHelper = shortStringHelper; + _passwordChanger = passwordChanger; + _scopeProvider = scopeProvider; + _twoFactorLoginService = twoFactorLoginService; + } + + [Obsolete("Use constructor that also takes an ITwoFactorLoginService. Scheduled for removal in V13")] public MemberController( ICultureDictionary cultureDictionary, ILoggerFactory loggerFactory, @@ -87,20 +127,24 @@ public class MemberController : ContentControllerBase IJsonSerializer jsonSerializer, IPasswordChanger passwordChanger, ICoreScopeProvider scopeProvider) - : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer) + : this( + cultureDictionary, + loggerFactory, + shortStringHelper, + eventMessages, + localizedTextService, + propertyEditors, + umbracoMapper, + memberService, + memberTypeService, + memberManager, + dataTypeService, + backOfficeSecurityAccessor, + jsonSerializer, + passwordChanger, + scopeProvider, + StaticServiceProvider.Instance.GetRequiredService()) { - _propertyEditors = propertyEditors; - _umbracoMapper = umbracoMapper; - _memberService = memberService; - _memberTypeService = memberTypeService; - _memberManager = memberManager; - _dataTypeService = dataTypeService; - _localizedTextService = localizedTextService; - _backOfficeSecurityAccessor = backOfficeSecurityAccessor; - _jsonSerializer = jsonSerializer; - _shortStringHelper = shortStringHelper; - _passwordChanger = passwordChanger; - _scopeProvider = scopeProvider; } /// @@ -540,6 +584,16 @@ public class MemberController : ContentControllerBase return ValidationProblem("An admin cannot lock a member"); } + // Handle disabling of 2FA + if (!contentItem.IsTwoFactorEnabled) + { + IEnumerable providers = await _twoFactorLoginService.GetEnabledTwoFactorProviderNamesAsync(contentItem.Key); + foreach (var provider in providers) + { + await _twoFactorLoginService.DisableAsync(contentItem.Key, provider); + } + } + // If we're changing the password... // Handle changing with the member manager & password changer (takes care of other nuances) if (contentItem.Password != null) 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 4c9ce4f61e..30b456f15b 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 @@ -299,6 +299,9 @@ case '_umb_lockedOut': saveModel.isLockedOut = prop.value; break; + case '_umb_twoFactorEnabled': + saveModel.isTwoFactorEnabled = prop.value; + break; } }); diff --git a/src/Umbraco.Web.Website/Controllers/UmbTwoFactorLoginController.cs b/src/Umbraco.Web.Website/Controllers/UmbTwoFactorLoginController.cs index 47b4389cf7..f498189ff0 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbTwoFactorLoginController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbTwoFactorLoginController.cs @@ -84,7 +84,7 @@ public class UmbTwoFactorLoginController : SurfaceController model.Code, model.IsPersistent, model.RememberClient); - if (result.Succeeded && returnUrl is not null) + if (result.Succeeded) { return RedirectToLocal(returnUrl); } 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 cc620940cf..83d39a4245 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -75,7 +75,8 @@ public class MemberControllerUnitTests IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange SetupMemberTestData(out var fakeMemberData, out _, ContentSaveAction.SaveNew); @@ -87,7 +88,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); sut.ModelState.AddModelError("key", "Invalid model state"); Mock.Get(umbracoMembersUserManager) @@ -119,7 +121,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var member = SetupMemberTestData(out var fakeMemberData, out var memberDisplay, ContentSaveAction.SaveNew); @@ -148,7 +151,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -171,7 +175,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var member = SetupMemberTestData(out var fakeMemberData, out var memberDisplay, ContentSaveAction.SaveNew); @@ -200,7 +205,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -223,7 +229,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var member = SetupMemberTestData(out var fakeMemberData, out var memberDisplay, ContentSaveAction.Save); @@ -262,7 +269,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -285,7 +293,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var member = SetupMemberTestData(out var fakeMemberData, out _, ContentSaveAction.Save); @@ -320,7 +329,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -375,7 +385,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var member = SetupMemberTestData(out var fakeMemberData, out _, ContentSaveAction.SaveNew); @@ -403,7 +414,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = sut.PostSave(fakeMemberData).Result; @@ -427,7 +439,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var roleName = "anyrole"; @@ -470,7 +483,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -509,7 +523,8 @@ public class MemberControllerUnitTests IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IPasswordChanger passwordChanger, - IOptions globalSettings) + IOptions globalSettings, + ITwoFactorLoginService twoFactorLoginService) { var httpContextAccessor = new HttpContextAccessor(); @@ -559,7 +574,8 @@ public class MemberControllerUnitTests memberGroupService, mockPasswordConfig.Object, contentTypeBaseServiceProvider.Object, - propertyEditorCollection)); + propertyEditorCollection, + twoFactorLoginService)); var map = new MapDefinitionCollection(() => new List { @@ -608,7 +624,8 @@ public class MemberControllerUnitTests backOfficeSecurityAccessor, new ConfigurationEditorJsonSerializer(), passwordChanger, - scopeProvider); + scopeProvider, + twoFactorLoginService); } /// @@ -699,6 +716,11 @@ public class MemberControllerUnitTests $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lockedOut", }, new() + { + Alias = + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}twoFactorEnabled", + }, + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate",