yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enabling an Umbraco admin user to disable 2FA for a member (#13369)

Browse Source 
* Fix Invalid authentication code bug

* Add translation keys for 2fa

* Display toggle for 2FA on member

* Add TwoFactorEnabled prop when saving member

* Handle disabling of 2FA

* Fix tests

* Changing obsolete msg

(cherry picked from commit af6b8fc5cb)
This commit is contained in:
Elitsa Marinovska
2022-11-28 13:42:38 +01:00
committed by Sebastiaan Janssen
parent d6d4421a37
commit dabc7bed9b
8 changed files with 170 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/Umbraco.Core/EmbeddedResources/Lang/en.xml
View File

@@ -346,6 +346,7 @@
<key alias="createNewMember">Create a new member</key>
<key alias="allMembers">All Members</key>
<key alias="memberGroupNoProperties">Member groups have no additional properties for editing.</key>
<key alias="2fa">Two-Factor Authentication</key>
</area>
<area alias="contentType">
<key alias="copyFailed">Failed to copy content type</key>

1
src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml
View File

@@ -362,6 +362,7 @@
<key alias="memberHasPassword">The member already has a password set</key>
<key alias="memberLockoutNotEnabled">Lockout is not enabled for this member</key>
<key alias="memberNotInGroup">The member is not in group '%0%'</key>
<key alias="2fa">Two-Factor Authentication</key>
</area>
<area alias="contentType">
<key alias="copyFailed">Failed to copy content type</key>

3
src/Umbraco.Core/Models/ContentEditing/MemberSave.cs
View File

@@ -34,6 +34,9 @@ public class MemberSave : ContentBaseSave<IMember>
[DataMember(Name = "isApproved")]
public bool IsApproved { get; set; }
[DataMember(Name = "isTwoFactorEnabled")]
public bool IsTwoFactorEnabled { get; set; }
private T? GetPropertyValue<T>(string alias)
{
ContentPropertyBasic? prop = Properties.FirstOrDefault(x => x.Alias == alias);

64
src/Umbraco.Core/Models/Mapping/MemberTabsAndPropertiesMapper.cs
View File

@@ -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<IMember>
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<MemberPasswordConfigurationSettings> 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<IMember>
IOptions<MemberPasswordConfigurationSettings> memberPasswordConfiguration,
IContentTypeBaseServiceProvider contentTypeBaseServiceProvider,
PropertyEditorCollection propertyEditorCollection)
: base(cultureDictionary, localizedTextService, contentTypeBaseServiceProvider)
: this(
cultureDictionary,
backofficeSecurityAccessor,
localizedTextService,
memberTypeService,
memberService,
memberGroupService,
memberPasswordConfiguration,
contentTypeBaseServiceProvider,
propertyEditorCollection,
StaticServiceProvider.Instance.GetRequiredService<ITwoFactorLoginService>())
{
_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;
}
/// <inheritdoc />
@@ -180,6 +213,8 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper<IMember>
public IEnumerable<ContentPropertyDisplay> MapMembershipProperties(IMember member, MapperContext? context)
{
var isTwoFactorEnabled = _twoFactorLoginService.IsTwoFactorEnabledAsync(member.Key).Result;
var properties = new List<ContentPropertyDisplay>
{
GetLoginProperty(member, _localizedTextService),
@@ -245,6 +280,17 @@ public class MemberTabsAndPropertiesMapper : TabsAndPropertiesMapper<IMember>
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",

80
src/Umbraco.Web.BackOffice/Controllers/MemberController.cs
View File

@@ -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<MemberIdentityUser> _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
/// <param name="jsonSerializer">The JSON serializer</param>
/// <param name="passwordChanger">The password changer</param>
/// <param name="scopeProvider">The core scope provider</param>
/// <param name="twoFactorLoginService">The two factor login service</param>
[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<MemberIdentityUser> 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<MemberIdentityUser> 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<ITwoFactorLoginService>())
{
_propertyEditors = propertyEditors;
_umbracoMapper = umbracoMapper;
_memberService = memberService;
_memberTypeService = memberTypeService;
_memberManager = memberManager;
_dataTypeService = dataTypeService;
_localizedTextService = localizedTextService;
_backOfficeSecurityAccessor = backOfficeSecurityAccessor;
_jsonSerializer = jsonSerializer;
_shortStringHelper = shortStringHelper;
_passwordChanger = passwordChanger;
_scopeProvider = scopeProvider;
}
/// <summary>
@@ -540,6 +584,16 @@ public class MemberController : ContentControllerBase
return ValidationProblem("An admin cannot lock a member");
}
// Handle disabling of 2FA
if (!contentItem.IsTwoFactorEnabled)
{
IEnumerable<string> 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)

3
src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js
View File

@@ -299,6 +299,9 @@
case '_umb_lockedOut':
saveModel.isLockedOut = prop.value;
break;
case '_umb_twoFactorEnabled':
saveModel.isTwoFactorEnabled = prop.value;
break;
}
});

2
src/Umbraco.Web.Website/Controllers/UmbTwoFactorLoginController.cs
View File

@@ -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);
}

56
tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs
View File

@@ -75,7 +75,8 @@ public class MemberControllerUnitTests
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> 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<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings)
IOptions<GlobalSettings> 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<IMapDefinition>
{
@@ -608,7 +624,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
new ConfigurationEditorJsonSerializer(),
passwordChanger,
scopeProvider);
scopeProvider,
twoFactorLoginService);
}
/// <summary>
@@ -699,6 +716,11 @@ public class MemberControllerUnitTests
$"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lockedOut",
},
new()
{
Alias =
$"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}twoFactorEnabled",
},
new()
{
Alias =
$"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}lastLockoutDate",