diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml index a236893a2c..6f99a1020f 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml @@ -345,6 +345,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 78484ac87d..65ef7b9f72 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml @@ -361,6 +361,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 65db6181dd..3c20babecc 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; } /// @@ -181,6 +214,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), @@ -246,6 +281,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 b7220a3941..f06ae05f86 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; } /// @@ -544,6 +588,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 c3fc359cf5..c0dbb05f82 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -74,7 +74,8 @@ public class MemberControllerUnitTests IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange SetupMemberTestData(out var fakeMemberData, out _, ContentSaveAction.SaveNew); @@ -86,7 +87,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); sut.ModelState.AddModelError("key", "Invalid model state"); Mock.Get(umbracoMembersUserManager) @@ -118,7 +120,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); @@ -147,7 +150,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -170,7 +174,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); @@ -199,7 +204,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -222,7 +228,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); @@ -261,7 +268,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -284,7 +292,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); @@ -319,7 +328,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -374,7 +384,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); @@ -402,7 +413,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = sut.PostSave(fakeMemberData).Result; @@ -426,7 +438,8 @@ public class MemberControllerUnitTests IBackOfficeSecurity backOfficeSecurity, IPasswordChanger passwordChanger, IOptions globalSettings, - IUser user) + IUser user, + ITwoFactorLoginService twoFactorLoginService) { // arrange var roleName = "anyrole"; @@ -469,7 +482,8 @@ public class MemberControllerUnitTests dataTypeService, backOfficeSecurityAccessor, passwordChanger, - globalSettings); + globalSettings, + twoFactorLoginService); // act var result = await sut.PostSave(fakeMemberData); @@ -508,7 +522,8 @@ public class MemberControllerUnitTests IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IPasswordChanger passwordChanger, - IOptions globalSettings) + IOptions globalSettings, + ITwoFactorLoginService twoFactorLoginService) { var httpContextAccessor = new HttpContextAccessor(); @@ -558,7 +573,8 @@ public class MemberControllerUnitTests memberGroupService, mockPasswordConfig.Object, contentTypeBaseServiceProvider.Object, - propertyEditorCollection)); + propertyEditorCollection, + twoFactorLoginService)); var map = new MapDefinitionCollection(() => new List { @@ -607,7 +623,8 @@ public class MemberControllerUnitTests backOfficeSecurityAccessor, new ConfigurationEditorJsonSerializer(), passwordChanger, - scopeProvider); + scopeProvider, + twoFactorLoginService); } /// @@ -698,6 +715,11 @@ public class MemberControllerUnitTests $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lockedOut", }, new() + { + Alias = + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}twoFactorEnabled", + }, + new() { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate",