From 79a0073a2c7e54fc7f4cd4af78a82ffbefab52bb Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Mon, 26 May 2025 16:28:43 +1000 Subject: [PATCH] Clean up --- test/Core.Test/Services/UserServiceTests.cs | 86 +++++++++++++-------- 1 file changed, 55 insertions(+), 31 deletions(-) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 9b3fb3d4ba..7845fa9f11 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -26,7 +26,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Options; using NSubstitute; -using NSubstitute.ReceivedExtensions; using Xunit; namespace Bit.Core.Test.Services; @@ -167,9 +166,13 @@ public class UserServiceTests [Theory] [BitAutoData(DeviceType.UnknownBrowser, "Unknown Browser")] [BitAutoData(DeviceType.Android, "Android")] - public async Task SendNewDeviceVerificationEmailAsync_DeviceMatches(DeviceType deviceType, string deviceTypeName, SutProvider sutProvider, User user) + public async Task SendNewDeviceVerificationEmailAsync_DeviceMatches(DeviceType deviceType, string deviceTypeName, + User user) { - // WithFakeTokenProvider(sutProvider, user); + var sutProvider = new SutProvider() + .WithFakeTokenProvider(user) + .Create(); + var context = sutProvider.GetDependency(); context.DeviceType = deviceType; context.IpAddress = "1.1.1.1"; @@ -182,9 +185,12 @@ public class UserServiceTests } [Theory, BitAutoData] - public async Task SendNewDeviceVerificationEmailAsync_NullDeviceTypeShouldSendUnkownBrowserType(SutProvider sutProvider, User user) + public async Task SendNewDeviceVerificationEmailAsync_NullDeviceTypeShouldSendUnkownBrowserType(User user) { - // WithFakeTokenProvider(sutProvider, user); + var sutProvider = new SutProvider() + .WithFakeTokenProvider(user) + .Create(); + var context = sutProvider.GetDependency(); context.DeviceType = null; context.IpAddress = "1.1.1.1"; @@ -259,29 +265,21 @@ public class UserServiceTests // Arrange SetupUserAndDevice(user, shouldHavePassword); + var sutProvider = new SutProvider() + .WithUserPasswordStore() + .WithFakeTokenProvider(user) + .Create() + .FixPasswordHasherBug(); + // Setup the fake password verification - var substitutedUserPasswordStore = Substitute.For>(); - substitutedUserPasswordStore + sutProvider.GetDependency>() .GetPasswordHashAsync(user, Arg.Any()) .Returns(Task.FromResult("hashed_test_password")); - var sutProvider = new SutProvider() - // IUserPasswordStore must be registered under the IUserStore parameter - .SetDependency>(substitutedUserPasswordStore) - .WithFakeTokenProvider(user) - .Create(); - sutProvider.GetDependency>() .VerifyHashedPassword(user, "hashed_test_password", "test_password") .Returns(PasswordVerificationResult.Success); - // HACK: reassign public property on base class after it's overwritten by autofixture - sutProvider.Sut.PasswordHasher = sutProvider.GetDependency>(); - - // DEBUG: check the public property on the base class matches the mock in SutProvider. - // If you remove the HACK above, this will fail (and so will the rest of the test). - Assert.Equal(sutProvider.Sut.PasswordHasher, sutProvider.GetDependency>()); - var actualIsVerified = await sutProvider.Sut.VerifySecretAsync(user, secret); Assert.Equal(expectedIsVerified, actualIsVerified); @@ -493,25 +491,28 @@ public class UserServiceTests } [Theory, BitAutoData] - public async Task ResendNewDeviceVerificationEmail_SendsToken_Success( - SutProvider sutProvider, User user) + public async Task ResendNewDeviceVerificationEmail_SendsToken_Success(User user) { // Arrange var testPassword = "test_password"; SetupUserAndDevice(user, true); + var sutProvider = new SutProvider() + .WithUserPasswordStore() + .WithFakeTokenProvider(user) + .Create() + .FixPasswordHasherBug(); + // Setup the fake password verification - var substitutedUserPasswordStore = Substitute.For>(); - substitutedUserPasswordStore + sutProvider + .GetDependency>() .GetPasswordHashAsync(user, Arg.Any()) .Returns((ci) => { return Task.FromResult("hashed_test_password"); }); - sutProvider.SetDependency>(substitutedUserPasswordStore, "store"); - - sutProvider.GetDependency>("passwordHasher") + sutProvider.GetDependency>() .VerifyHashedPassword(user, "hashed_test_password", testPassword) .Returns((ci) => { @@ -526,10 +527,7 @@ public class UserServiceTests context.DeviceType = DeviceType.Android; context.IpAddress = "1.1.1.1"; - // HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured - var sut = sutProvider.Sut; - - await sut.ResendNewDeviceVerificationEmail(user.Email, testPassword); + await sutProvider.Sut.ResendNewDeviceVerificationEmail(user.Email, testPassword); await sutProvider.GetDependency() .Received(1) @@ -721,4 +719,30 @@ public static class UserServiceSutProviderExtensions return sutProvider; } + + public static SutProvider WithUserPasswordStore(this SutProvider sutProvider) + { + var substitutedUserPasswordStore = Substitute.For>(); + + // IUserPasswordStore must be registered under the IUserStore parameter to be properly injected + sutProvider.SetDependency>(substitutedUserPasswordStore); + + // also store it under its own type for retrieval and configuration + sutProvider.SetDependency(substitutedUserPasswordStore); + + return sutProvider; + } + + /// + /// This is a hack: when autofixture initializes the sut in sutProvider, it overwrites the public + /// PasswordHasher property with a new substitute, so it loses the configured sutProvider mock. + /// This doesn't usually happen because our dependencies are not usually public. + /// Call this AFTER SutProvider.Create(). + /// + public static SutProvider FixPasswordHasherBug(this SutProvider sutProvider) + { + // Get the configured sutProvider mock and assign it back to the public property in the base class + sutProvider.Sut.PasswordHasher = sutProvider.GetDependency>(); + return sutProvider; + } }