Review: Allow Duplicate Email for Members (#16202)

* init

* Aligned default values on security settings.

* Added validator for security settings.

* Provide default implementation for get members by email.

* Refactored constructor of MemberController.

* Validate on unique member email only when configured to do so.

* Further code tidy and use of DI in constructor.

* Used new constructor in tests.

* Add unit test for modified behaviour.

* Removed validator for security settings (it's not necessary, I got confused with users and members).

* Spelling.

---------

Co-authored-by: Andy Butland <abutland73@gmail.com>
This commit is contained in:
jasont0101
2025-02-05 03:38:40 -08:00
committed by GitHub
parent cfb0fc23ac
commit 095a73132c
8 changed files with 219 additions and 49 deletions

View File

@@ -70,11 +70,13 @@ public class MemberControllerUnitTests
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
IUser user,
ITwoFactorLoginService twoFactorLoginService)
{
// arrange
SetupMemberTestData(out var fakeMemberData, out _, ContentSaveAction.SaveNew);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -84,7 +86,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
sut.ModelState.AddModelError("key", "Invalid model state");
Mock.Get(umbracoMembersUserManager)
@@ -116,7 +119,6 @@ public class MemberControllerUnitTests
IBackOfficeSecurity backOfficeSecurity,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
IUser user,
ITwoFactorLoginService twoFactorLoginService)
{
// arrange
@@ -138,6 +140,8 @@ public class MemberControllerUnitTests
.Returns(() => member);
Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny<string>())).Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -147,7 +151,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = await sut.PostSave(fakeMemberData);
@@ -170,7 +175,6 @@ public class MemberControllerUnitTests
IBackOfficeSecurity backOfficeSecurity,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
IUser user,
ITwoFactorLoginService twoFactorLoginService)
{
// arrange
@@ -192,6 +196,8 @@ public class MemberControllerUnitTests
.Returns(() => member);
Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny<string>())).Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -201,7 +207,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = await sut.PostSave(fakeMemberData);
@@ -256,6 +263,8 @@ public class MemberControllerUnitTests
.Returns(() => null)
.Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -265,7 +274,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = await sut.PostSave(fakeMemberData);
@@ -316,6 +326,8 @@ public class MemberControllerUnitTests
.Returns(() => null)
.Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -325,7 +337,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = await sut.PostSave(fakeMemberData);
@@ -382,7 +395,6 @@ public class MemberControllerUnitTests
IBackOfficeSecurity backOfficeSecurity,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
IUser user,
ITwoFactorLoginService twoFactorLoginService)
{
// arrange
@@ -403,6 +415,8 @@ public class MemberControllerUnitTests
x => x.GetByEmail(It.IsAny<string>()))
.Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -412,7 +426,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = sut.PostSave(fakeMemberData).Result;
@@ -424,6 +439,66 @@ public class MemberControllerUnitTests
Assert.AreEqual(StatusCodes.Status400BadRequest, validation?.StatusCode);
}
[Test]
[AutoMoqData]
public void PostSaveMember_SaveNew_WhenMemberEmailAlreadyExists_AndDuplicateEmailsAreAllowed_ExpectSuccessResponse(
[Frozen] IMemberManager umbracoMembersUserManager,
IMemberService memberService,
IMemberTypeService memberTypeService,
IMemberGroupService memberGroupService,
IDataTypeService dataTypeService,
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IBackOfficeSecurity backOfficeSecurity,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
ITwoFactorLoginService twoFactorLoginService)
{
// arrange
var member = SetupMemberTestData(out var fakeMemberData, out var memberDisplay, ContentSaveAction.SaveNew);
Mock.Get(umbracoMembersUserManager)
.Setup(x => x.CreateAsync(It.IsAny<MemberIdentityUser>(), It.IsAny<string>()))
.ReturnsAsync(() => IdentityResult.Success);
Mock.Get(umbracoMembersUserManager)
.Setup(x => x.ValidatePasswordAsync(It.IsAny<string>()))
.ReturnsAsync(() => IdentityResult.Success);
Mock.Get(umbracoMembersUserManager)
.Setup(x => x.GetRolesAsync(It.IsAny<MemberIdentityUser>()))
.ReturnsAsync(() => Array.Empty<string>());
Mock.Get(memberTypeService).Setup(x => x.GetDefault()).Returns("fakeAlias");
Mock.Get(backOfficeSecurityAccessor).Setup(x => x.BackOfficeSecurity).Returns(backOfficeSecurity);
Mock.Get(memberService).SetupSequence(
x => x.GetByEmail(It.IsAny<string>()))
.Returns(() => null)
.Returns(() => member);
Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny<string>())).Returns(() => member);
Mock.Get(memberService).SetupSequence(
x => x.GetByEmail(It.IsAny<string>()))
.Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings { MemberRequireUniqueEmail = false });
var sut = CreateSut(
memberService,
memberTypeService,
memberGroupService,
umbracoMembersUserManager,
dataTypeService,
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService,
securitySettings);
// act
var result = sut.PostSave(fakeMemberData).Result;
var validation = result.Result as ValidationErrorResult;
// assert
Assert.IsNull(result.Result);
Assert.IsNotNull(result.Value);
}
[Test]
[AutoMoqData]
public async Task PostSaveMember_SaveExistingMember_WithNoRoles_Add1Role_ExpectSuccessResponse(
@@ -472,6 +547,9 @@ public class MemberControllerUnitTests
x => x.GetByEmail(It.IsAny<string>()))
.Returns(() => null)
.Returns(() => member);
var securitySettings = Options.Create(new SecuritySettings());
var sut = CreateSut(
memberService,
memberTypeService,
@@ -481,7 +559,8 @@ public class MemberControllerUnitTests
backOfficeSecurityAccessor,
passwordChanger,
globalSettings,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
// act
var result = await sut.PostSave(fakeMemberData);
@@ -512,6 +591,7 @@ public class MemberControllerUnitTests
/// <param name="passwordChanger">Password changer class</param>
/// <param name="globalSettings">The global settings</param>
/// <param name="twoFactorLoginService">The two factor login service</param>
/// <param name="securitySettings">The security settings</param>
/// <returns>A member controller for the tests</returns>
private MemberController CreateSut(
IMemberService memberService,
@@ -522,7 +602,8 @@ public class MemberControllerUnitTests
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IPasswordChanger<MemberIdentityUser> passwordChanger,
IOptions<GlobalSettings> globalSettings,
ITwoFactorLoginService twoFactorLoginService)
ITwoFactorLoginService twoFactorLoginService,
IOptions<SecuritySettings> securitySettings)
{
var httpContextAccessor = new HttpContextAccessor();
@@ -623,7 +704,8 @@ public class MemberControllerUnitTests
new ConfigurationEditorJsonSerializer(),
passwordChanger,
scopeProvider,
twoFactorLoginService);
twoFactorLoginService,
securitySettings);
}
/// <summary>