From fb9fa4afe21dc328c7ad660a06994601ed5a252c Mon Sep 17 00:00:00 2001 From: Fares Amr Date: Wed, 1 Sep 2021 01:05:45 +0200 Subject: [PATCH] Remove unnecessary normalization in RoleExistsAsync method and update unit test with custom normalizer --- .../Extensions.Core/src/RoleManager.cs | 2 +- .../test/Identity.Test/RoleManagerTest.cs | 12 ++-- .../test/Identity.Test/UserManagerTest.cs | 65 +++++++++++-------- src/Identity/test/Shared/MockHelpers.cs | 20 +++++- 4 files changed, 64 insertions(+), 35 deletions(-) diff --git a/src/Identity/Extensions.Core/src/RoleManager.cs b/src/Identity/Extensions.Core/src/RoleManager.cs index ea75b9476a87..bb2cdc415d60 100644 --- a/src/Identity/Extensions.Core/src/RoleManager.cs +++ b/src/Identity/Extensions.Core/src/RoleManager.cs @@ -234,7 +234,7 @@ public virtual async Task RoleExistsAsync(string roleName) throw new ArgumentNullException(nameof(roleName)); } - return await FindByNameAsync(NormalizeKey(roleName)) != null; + return await FindByNameAsync(roleName) != null; } /// diff --git a/src/Identity/test/Identity.Test/RoleManagerTest.cs b/src/Identity/test/Identity.Test/RoleManagerTest.cs index 1ed760a2a4ea..74eb372b20ee 100644 --- a/src/Identity/test/Identity.Test/RoleManagerTest.cs +++ b/src/Identity/test/Identity.Test/RoleManagerTest.cs @@ -17,11 +17,12 @@ public class RoleManagerTest public async Task CreateCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var role = new PocoRole { Name = "Foo" }; store.Setup(s => s.CreateAsync(role, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); store.Setup(s => s.GetRoleNameAsync(role, CancellationToken.None)).Returns(Task.FromResult(role.Name)).Verifiable(); - store.Setup(s => s.SetNormalizedRoleNameAsync(role, role.Name.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedRoleNameAsync(role, normalizer.NormalizeName(role.Name), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); var roleManager = MockHelpers.TestRoleManager(store.Object); // Act @@ -36,11 +37,12 @@ public async Task CreateCallsStore() public async Task UpdateCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var role = new PocoRole { Name = "Foo" }; store.Setup(s => s.UpdateAsync(role, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); store.Setup(s => s.GetRoleNameAsync(role, CancellationToken.None)).Returns(Task.FromResult(role.Name)).Verifiable(); - store.Setup(s => s.SetNormalizedRoleNameAsync(role, role.Name.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedRoleNameAsync(role, normalizer.NormalizeName(role.Name), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); var roleManager = MockHelpers.TestRoleManager(store.Object); // Act @@ -63,9 +65,10 @@ public void RolesQueryableFailWhenStoreNotImplemented() public async Task FindByNameCallsStoreWithNormalizedName() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var role = new PocoRole { Name = "Foo" }; - store.Setup(s => s.FindByNameAsync("FOO", CancellationToken.None)).Returns(Task.FromResult(role)).Verifiable(); + store.Setup(s => s.FindByNameAsync(normalizer.NormalizeName("Foo"), CancellationToken.None)).Returns(Task.FromResult(role)).Verifiable(); var manager = MockHelpers.TestRoleManager(store.Object); // Act @@ -98,9 +101,10 @@ public async Task CanFindByNameCallsStoreWithoutNormalizedName() public async Task RoleExistsCallsStoreWithNormalizedName() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var role = new PocoRole { Name = "Foo" }; - store.Setup(s => s.FindByNameAsync("FOO", CancellationToken.None)).Returns(Task.FromResult(role)).Verifiable(); + store.Setup(s => s.FindByNameAsync(normalizer.NormalizeName("Foo"), CancellationToken.None)).Returns(Task.FromResult(role)).Verifiable(); var manager = MockHelpers.TestRoleManager(store.Object); // Act diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 57061d3164f2..1dbb424b0d20 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -70,11 +70,12 @@ public CustomRoleManager() : base(new Mock>().Object, null, public async Task CreateCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; store.Setup(s => s.CreateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); store.Setup(s => s.GetUserNameAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.UserName)).Verifiable(); - store.Setup(s => s.SetNormalizedUserNameAsync(user, user.UserName.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedUserNameAsync(user, normalizer.NormalizeName(user.UserName), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); // Act @@ -108,13 +109,14 @@ public async Task CreateUpdatesSecurityStampStore() public async Task CreateCallsUpdateEmailStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo", Email = "Foo@foo.com" }; store.Setup(s => s.CreateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); store.Setup(s => s.GetUserNameAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.UserName)).Verifiable(); store.Setup(s => s.GetEmailAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.Email)).Verifiable(); - store.Setup(s => s.SetNormalizedEmailAsync(user, user.Email.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); - store.Setup(s => s.SetNormalizedUserNameAsync(user, user.UserName.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedEmailAsync(user, normalizer.NormalizeEmail(user.Email), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedUserNameAsync(user, normalizer.NormalizeName(user.UserName), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); // Act @@ -146,10 +148,11 @@ public async Task DeleteCallsStore() public async Task UpdateCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; store.Setup(s => s.GetUserNameAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.UserName)).Verifiable(); - store.Setup(s => s.SetNormalizedUserNameAsync(user, user.UserName.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedUserNameAsync(user, normalizer.NormalizeName(user.UserName), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -165,12 +168,13 @@ public async Task UpdateCallsStore() public async Task UpdateWillUpdateNormalizedEmail() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo", Email = "email" }; store.Setup(s => s.GetUserNameAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.UserName)).Verifiable(); store.Setup(s => s.GetEmailAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.Email)).Verifiable(); - store.Setup(s => s.SetNormalizedUserNameAsync(user, user.UserName.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); - store.Setup(s => s.SetNormalizedEmailAsync(user, user.Email.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedUserNameAsync(user, normalizer.NormalizeName(user.UserName), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedEmailAsync(user, normalizer.NormalizeEmail(user.Email), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -186,11 +190,12 @@ public async Task UpdateWillUpdateNormalizedEmail() public async Task SetUserNameCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser(); store.Setup(s => s.SetUserNameAsync(user, "foo", CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); store.Setup(s => s.GetUserNameAsync(user, CancellationToken.None)).Returns(Task.FromResult("foo")).Verifiable(); - store.Setup(s => s.SetNormalizedUserNameAsync(user, "FOO", CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); + store.Setup(s => s.SetNormalizedUserNameAsync(user, normalizer.NormalizeName("foo"), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable(); store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -223,9 +228,10 @@ public async Task FindByIdCallsStore() public async Task FindByNameCallsStoreWithNormalizedName() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; - store.Setup(s => s.FindByNameAsync(user.UserName.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); + store.Setup(s => s.FindByNameAsync(normalizer.NormalizeName(user.UserName), CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); // Act @@ -258,9 +264,10 @@ public async Task CanFindByNameCallsStoreWithoutNormalizedName() public async Task FindByEmailCallsStoreWithNormalizedEmail() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { Email = "Foo" }; - store.Setup(s => s.FindByEmailAsync(user.Email.ToUpperInvariant(), CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); + store.Setup(s => s.FindByEmailAsync(normalizer.NormalizeEmail(user.Email), CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); // Act @@ -335,27 +342,28 @@ public async Task FindByNameCallsStoreWithCustomNormalizedName() public async Task AddToRolesCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; var roles = new string[] { "A", "B", "C", "C" }; - store.Setup(s => s.AddToRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.AddToRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.AddToRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.AddToRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.AddToRoleAsync(user, "C", CancellationToken.None)) + store.Setup(s => s.AddToRoleAsync(user, normalizer.NormalizeName("C"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(false)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(false)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "C", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("C"), CancellationToken.None)) .Returns(Task.FromResult(false)) .Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -366,7 +374,7 @@ public async Task AddToRolesCallsStore() // Assert Assert.True(result.Succeeded); store.VerifyAll(); - store.Verify(s => s.AddToRoleAsync(user, "C", CancellationToken.None), Times.Once()); + store.Verify(s => s.AddToRoleAsync(user, normalizer.NormalizeName("C"), CancellationToken.None), Times.Once()); } [Fact] @@ -412,13 +420,14 @@ public async Task AddToRolesCallsStoreWithCustomNameNormalizer() public async Task AddToRolesFailsIfUserInRole() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; var roles = new[] { "A", "B", "C" }; - store.Setup(s => s.AddToRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.AddToRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(true)) .Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -435,26 +444,27 @@ public async Task AddToRolesFailsIfUserInRole() public async Task RemoveFromRolesCallsStore() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; var roles = new[] { "A", "B", "C" }; - store.Setup(s => s.RemoveFromRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.RemoveFromRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.RemoveFromRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.RemoveFromRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.RemoveFromRoleAsync(user, "C", CancellationToken.None)) + store.Setup(s => s.RemoveFromRoleAsync(user, normalizer.NormalizeName("C"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(true)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(true)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "C", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("C"), CancellationToken.None)) .Returns(Task.FromResult(true)) .Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); @@ -471,16 +481,17 @@ public async Task RemoveFromRolesCallsStore() public async Task RemoveFromRolesFailsIfNotInRole() { // Setup + var normalizer = MockHelpers.MockLookupNormalizer(); var store = new Mock>(); var user = new PocoUser { UserName = "Foo" }; var roles = new string[] { "A", "B", "C" }; - store.Setup(s => s.RemoveFromRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.RemoveFromRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "A", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("A"), CancellationToken.None)) .Returns(Task.FromResult(true)) .Verifiable(); - store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + store.Setup(s => s.IsInRoleAsync(user, normalizer.NormalizeName("B"), CancellationToken.None)) .Returns(Task.FromResult(false)) .Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); diff --git a/src/Identity/test/Shared/MockHelpers.cs b/src/Identity/test/Shared/MockHelpers.cs index dac491995dd4..e2a5f291e401 100644 --- a/src/Identity/test/Shared/MockHelpers.cs +++ b/src/Identity/test/Shared/MockHelpers.cs @@ -30,7 +30,7 @@ public static Mock> MockRoleManager(IRoleStore store = store ?? new Mock>().Object; var roles = new List>(); roles.Add(new RoleValidator()); - return new Mock>(store, roles, new UpperInvariantLookupNormalizer(), + return new Mock>(store, roles, MockLookupNormalizer(), new IdentityErrorDescriber(), null); } @@ -47,7 +47,7 @@ public static UserManager TestUserManager(IUserStore store var pwdValidators = new List>(); pwdValidators.Add(new PasswordValidator()); var userManager = new UserManager(store, options.Object, new PasswordHasher(), - userValidators, pwdValidators, new UpperInvariantLookupNormalizer(), + userValidators, pwdValidators, MockLookupNormalizer(), new IdentityErrorDescriber(), null, new Mock>>().Object); validator.Setup(v => v.ValidateAsync(userManager, It.IsAny())) @@ -61,10 +61,24 @@ public static RoleManager TestRoleManager(IRoleStore store var roles = new List>(); roles.Add(new RoleValidator()); return new RoleManager(store, roles, - new UpperInvariantLookupNormalizer(), + MockLookupNormalizer(), new IdentityErrorDescriber(), null); } + public static ILookupNormalizer MockLookupNormalizer() + { + var normalizerFunc = new Func(i => + { + if (i == null) + return null; + else + return Convert.ToBase64String(Encoding.UTF8.GetBytes(i)).ToUpperInvariant(); + }); + var lookupNormalizer = new Mock(); + lookupNormalizer.Setup(i => i.NormalizeName(It.IsAny())).Returns(normalizerFunc); + lookupNormalizer.Setup(i => i.NormalizeEmail(It.IsAny())).Returns(normalizerFunc); + return lookupNormalizer.Object; + } } }