From 95ab2fa4af43c552bb05a83f3cc4916a51c33744 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 15 Mar 2019 14:00:40 -0700 Subject: [PATCH] Split ILookupNormalizer.Normalize into Name/Email methods (#8412) --- ....Extensions.Identity.Core.netcoreapp3.0.cs | 11 ++- .../Extensions.Core/src/ILookupNormalizer.cs | 20 +++-- .../Extensions.Core/src/RoleManager.cs | 2 +- .../src/UpperInvariantLookupNormalizer.cs | 22 +++-- .../Extensions.Core/src/UserManager.cs | 48 ++++++----- .../test/Identity.Test/RoleManagerTest.cs | 3 +- .../test/Identity.Test/UserManagerTest.cs | 83 ++++++++++++++++++- 7 files changed, 146 insertions(+), 43 deletions(-) diff --git a/src/Identity/Extensions.Core/ref/Microsoft.Extensions.Identity.Core.netcoreapp3.0.cs b/src/Identity/Extensions.Core/ref/Microsoft.Extensions.Identity.Core.netcoreapp3.0.cs index fed755ec7f..fce3a8894f 100644 --- a/src/Identity/Extensions.Core/ref/Microsoft.Extensions.Identity.Core.netcoreapp3.0.cs +++ b/src/Identity/Extensions.Core/ref/Microsoft.Extensions.Identity.Core.netcoreapp3.0.cs @@ -109,7 +109,8 @@ namespace Microsoft.AspNetCore.Identity } public partial interface ILookupNormalizer { - string Normalize(string key); + string NormalizeEmail(string email); + string NormalizeName(string name); } public partial interface ILookupProtector { @@ -450,10 +451,11 @@ namespace Microsoft.AspNetCore.Identity [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task ValidateAsync(string purpose, string token, Microsoft.AspNetCore.Identity.UserManager manager, TUser user) { throw null; } } - public partial class UpperInvariantLookupNormalizer : Microsoft.AspNetCore.Identity.ILookupNormalizer + public sealed partial class UpperInvariantLookupNormalizer : Microsoft.AspNetCore.Identity.ILookupNormalizer { public UpperInvariantLookupNormalizer() { } - public virtual string Normalize(string key) { throw null; } + public string NormalizeEmail(string email) { throw null; } + public string NormalizeName(string name) { throw null; } } public partial class UserClaimsPrincipalFactory : Microsoft.AspNetCore.Identity.IUserClaimsPrincipalFactory where TUser : class { @@ -600,7 +602,8 @@ namespace Microsoft.AspNetCore.Identity [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task IsLockedOutAsync(TUser user) { throw null; } public virtual System.Threading.Tasks.Task IsPhoneNumberConfirmedAsync(TUser user) { throw null; } - public virtual string NormalizeKey(string key) { throw null; } + public virtual string NormalizeEmail(string email) { throw null; } + public virtual string NormalizeName(string name) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task RedeemTwoFactorRecoveryCodeAsync(TUser user, string code) { throw null; } public virtual void RegisterTokenProvider(string providerName, Microsoft.AspNetCore.Identity.IUserTwoFactorTokenProvider provider) { } diff --git a/src/Identity/Extensions.Core/src/ILookupNormalizer.cs b/src/Identity/Extensions.Core/src/ILookupNormalizer.cs index 7cefc4a8a0..3bb5136880 100644 --- a/src/Identity/Extensions.Core/src/ILookupNormalizer.cs +++ b/src/Identity/Extensions.Core/src/ILookupNormalizer.cs @@ -4,15 +4,23 @@ namespace Microsoft.AspNetCore.Identity { /// - /// Provides an abstraction for normalizing keys for lookup purposes. + /// Provides an abstraction for normalizing keys (emails/names) for lookup purposes. /// public interface ILookupNormalizer { /// - /// Returns a normalized representation of the specified . + /// Returns a normalized representation of the specified . /// - /// The key to normalize. - /// A normalized representation of the specified . - string Normalize(string key); + /// The key to normalize. + /// A normalized representation of the specified . + string NormalizeName(string name); + + /// + /// Returns a normalized representation of the specified . + /// + /// The email to normalize. + /// A normalized representation of the specified . + string NormalizeEmail(string email); + } -} \ No newline at end of file +} diff --git a/src/Identity/Extensions.Core/src/RoleManager.cs b/src/Identity/Extensions.Core/src/RoleManager.cs index ac93b9941e..121d0d5b0a 100644 --- a/src/Identity/Extensions.Core/src/RoleManager.cs +++ b/src/Identity/Extensions.Core/src/RoleManager.cs @@ -244,7 +244,7 @@ namespace Microsoft.AspNetCore.Identity /// A normalized representation of the specified . public virtual string NormalizeKey(string key) { - return (KeyNormalizer == null) ? key : KeyNormalizer.Normalize(key); + return (KeyNormalizer == null) ? key : KeyNormalizer.NormalizeName(key); } /// diff --git a/src/Identity/Extensions.Core/src/UpperInvariantLookupNormalizer.cs b/src/Identity/Extensions.Core/src/UpperInvariantLookupNormalizer.cs index ec3fe26420..87aecfe825 100644 --- a/src/Identity/Extensions.Core/src/UpperInvariantLookupNormalizer.cs +++ b/src/Identity/Extensions.Core/src/UpperInvariantLookupNormalizer.cs @@ -6,21 +6,27 @@ namespace Microsoft.AspNetCore.Identity /// /// Implements by converting keys to their upper cased invariant culture representation. /// - public class UpperInvariantLookupNormalizer : ILookupNormalizer + public sealed class UpperInvariantLookupNormalizer : ILookupNormalizer { /// - /// Returns a normalized representation of the specified - /// by converting keys to their upper cased invariant culture representation. + /// Returns a normalized representation of the specified . /// - /// The key to normalize. - /// A normalized representation of the specified . - public virtual string Normalize(string key) + /// The key to normalize. + /// A normalized representation of the specified . + public string NormalizeName(string name) { - if (key == null) + if (name == null) { return null; } - return key.Normalize().ToUpperInvariant(); + return name.Normalize().ToUpperInvariant(); } + + /// + /// Returns a normalized representation of the specified . + /// + /// The email to normalize. + /// A normalized representation of the specified . + public string NormalizeEmail(string email) => NormalizeName(email); } } diff --git a/src/Identity/Extensions.Core/src/UserManager.cs b/src/Identity/Extensions.Core/src/UserManager.cs index 5d1be95124..821279b4a7 100644 --- a/src/Identity/Extensions.Core/src/UserManager.cs +++ b/src/Identity/Extensions.Core/src/UserManager.cs @@ -105,15 +105,15 @@ namespace Microsoft.AspNetCore.Identity foreach (var providerName in Options.Tokens.ProviderMap.Keys) { var description = Options.Tokens.ProviderMap[providerName]; - - var provider = (description.ProviderInstance ?? services.GetRequiredService(description.ProviderType)) + + var provider = (description.ProviderInstance ?? services.GetRequiredService(description.ProviderType)) as IUserTwoFactorTokenProvider; if (provider != null) { RegisterTokenProvider(providerName, provider); } } - } + } if (Options.Stores.ProtectPersonalData) { @@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.Identity /// The used to normalize things like user and role names. /// public ILookupNormalizer KeyNormalizer { get; set; } - + /// /// The used to generate error messages. /// @@ -547,7 +547,7 @@ namespace Microsoft.AspNetCore.Identity { throw new ArgumentNullException(nameof(userName)); } - userName = NormalizeKey(userName); + userName = NormalizeName(userName); var user = await Store.FindByNameAsync(userName, CancellationToken); @@ -603,15 +603,21 @@ namespace Microsoft.AspNetCore.Identity } /// - /// Normalize a key (user name, email) for consistent comparisons. + /// Normalize user or role name for consistent comparisons. /// - /// The key to normalize. - /// A normalized value representing the specified . - public virtual string NormalizeKey(string key) - { - return (KeyNormalizer == null) ? key : KeyNormalizer.Normalize(key); - } + /// The name to normalize. + /// A normalized value representing the specified . + public virtual string NormalizeName(string name) + => (KeyNormalizer == null) ? name : KeyNormalizer.NormalizeName(name); + /// + /// Normalize email for consistent comparisons. + /// + /// The email to normalize. + /// A normalized value representing the specified . + public virtual string NormalizeEmail(string email) + => (KeyNormalizer == null) ? email : KeyNormalizer.NormalizeEmail(email); + private string ProtectPersonalData(string data) { if (Options.Stores.ProtectPersonalData) @@ -630,7 +636,7 @@ namespace Microsoft.AspNetCore.Identity /// The that represents the asynchronous operation. public virtual async Task UpdateNormalizedUserNameAsync(TUser user) { - var normalizedName = NormalizeKey(await GetUserNameAsync(user)); + var normalizedName = NormalizeName(await GetUserNameAsync(user)); normalizedName = ProtectPersonalData(normalizedName); await Store.SetNormalizedUserNameAsync(user, normalizedName, CancellationToken); } @@ -1199,7 +1205,7 @@ namespace Microsoft.AspNetCore.Identity throw new ArgumentNullException(nameof(user)); } - var normalizedRole = NormalizeKey(role); + var normalizedRole = NormalizeName(role); if (await userRoleStore.IsInRoleAsync(user, normalizedRole, CancellationToken)) { return await UserAlreadyInRoleError(user, role); @@ -1232,7 +1238,7 @@ namespace Microsoft.AspNetCore.Identity foreach (var role in roles.Distinct()) { - var normalizedRole = NormalizeKey(role); + var normalizedRole = NormalizeName(role); if (await userRoleStore.IsInRoleAsync(user, normalizedRole, CancellationToken)) { return await UserAlreadyInRoleError(user, role); @@ -1260,7 +1266,7 @@ namespace Microsoft.AspNetCore.Identity throw new ArgumentNullException(nameof(user)); } - var normalizedRole = NormalizeKey(role); + var normalizedRole = NormalizeName(role); if (!await userRoleStore.IsInRoleAsync(user, normalizedRole, CancellationToken)) { return await UserNotInRoleError(user, role); @@ -1305,7 +1311,7 @@ namespace Microsoft.AspNetCore.Identity foreach (var role in roles) { - var normalizedRole = NormalizeKey(role); + var normalizedRole = NormalizeName(role); if (!await userRoleStore.IsInRoleAsync(user, normalizedRole, CancellationToken)) { return await UserNotInRoleError(user, role); @@ -1348,7 +1354,7 @@ namespace Microsoft.AspNetCore.Identity { throw new ArgumentNullException(nameof(user)); } - return await userRoleStore.IsInRoleAsync(user, NormalizeKey(role), CancellationToken); + return await userRoleStore.IsInRoleAsync(user, NormalizeName(role), CancellationToken); } /// @@ -1409,7 +1415,7 @@ namespace Microsoft.AspNetCore.Identity throw new ArgumentNullException(nameof(email)); } - email = NormalizeKey(email); + email = NormalizeEmail(email); var user = await store.FindByEmailAsync(email, CancellationToken); // Need to potentially check all keys @@ -1444,7 +1450,7 @@ namespace Microsoft.AspNetCore.Identity if (store != null) { var email = await GetEmailAsync(user); - await store.SetNormalizedEmailAsync(user, ProtectPersonalData(NormalizeKey(email)), CancellationToken); + await store.SetNormalizedEmailAsync(user, ProtectPersonalData(NormalizeEmail(email)), CancellationToken); } } @@ -2097,7 +2103,7 @@ namespace Microsoft.AspNetCore.Identity throw new ArgumentNullException(nameof(roleName)); } - return store.GetUsersInRoleAsync(NormalizeKey(roleName), CancellationToken); + return store.GetUsersInRoleAsync(NormalizeName(roleName), CancellationToken); } /// diff --git a/src/Identity/test/Identity.Test/RoleManagerTest.cs b/src/Identity/test/Identity.Test/RoleManagerTest.cs index acb16e9b3c..bebebc9ea0 100644 --- a/src/Identity/test/Identity.Test/RoleManagerTest.cs +++ b/src/Identity/test/Identity.Test/RoleManagerTest.cs @@ -111,7 +111,6 @@ namespace Microsoft.AspNetCore.Identity.Test store.VerifyAll(); } - [Fact] public void DisposeAfterDisposeDoesNotThrow() { @@ -209,4 +208,4 @@ namespace Microsoft.AspNetCore.Identity.Test } } } -} \ No newline at end of file +} diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 174f9bdc1a..91390f6968 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -269,6 +269,48 @@ namespace Microsoft.AspNetCore.Identity.Test store.VerifyAll(); } + private class CustomNormalizer : ILookupNormalizer + { + public string NormalizeName(string key) => "#"+key; + public string NormalizeEmail(string key) => "@" + key; + } + + [Fact] + public async Task FindByEmailCallsStoreWithCustomNormalizedEmail() + { + // Setup + var store = new Mock>(); + var user = new PocoUser { Email = "Foo" }; + store.Setup(s => s.FindByEmailAsync("@Foo", CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); + var userManager = MockHelpers.TestUserManager(store.Object); + userManager.KeyNormalizer = new CustomNormalizer(); + + // Act + var result = await userManager.FindByEmailAsync(user.Email); + + // Assert + Assert.Equal(user, result); + store.VerifyAll(); + } + + [Fact] + public async Task FindByNameCallsStoreWithCustomNormalizedName() + { + // Setup + var store = new Mock>(); + var user = new PocoUser { UserName = "Foo", Email = "Bar" }; + store.Setup(s => s.FindByNameAsync("#Foo", CancellationToken.None)).Returns(Task.FromResult(user)).Verifiable(); + var userManager = MockHelpers.TestUserManager(store.Object); + userManager.KeyNormalizer = new CustomNormalizer(); + + // Act + var result = await userManager.FindByNameAsync(user.UserName); + + // Assert + Assert.Equal(user, result); + store.VerifyAll(); + } + [Fact] public async Task AddToRolesCallsStore() { @@ -307,6 +349,45 @@ namespace Microsoft.AspNetCore.Identity.Test store.Verify(s => s.AddToRoleAsync(user, "C", CancellationToken.None), Times.Once()); } + [Fact] + public async Task AddToRolesCallsStoreWithCustomNameNormalizer() + { + // Setup + 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)) + .Returns(Task.FromResult(0)) + .Verifiable(); + store.Setup(s => s.AddToRoleAsync(user, "#B", CancellationToken.None)) + .Returns(Task.FromResult(0)) + .Verifiable(); + store.Setup(s => s.AddToRoleAsync(user, "#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)) + .Returns(Task.FromResult(false)) + .Verifiable(); + store.Setup(s => s.IsInRoleAsync(user, "#B", CancellationToken.None)) + .Returns(Task.FromResult(false)) + .Verifiable(); + store.Setup(s => s.IsInRoleAsync(user, "#C", CancellationToken.None)) + .Returns(Task.FromResult(false)) + .Verifiable(); + var userManager = MockHelpers.TestUserManager(store.Object); + userManager.KeyNormalizer = new CustomNormalizer(); + + // Act + var result = await userManager.AddToRolesAsync(user, roles); + + // Assert + Assert.True(result.Succeeded); + store.VerifyAll(); + store.Verify(s => s.AddToRoleAsync(user, "#C", CancellationToken.None), Times.Once()); + } + [Fact] public async Task AddToRolesFailsIfUserInRole() { @@ -1664,4 +1745,4 @@ namespace Microsoft.AspNetCore.Identity.Test } } -} \ No newline at end of file +}