From 87bbff65867710b5a49265fdf09de4a6d539ac8d Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 26 Oct 2017 12:00:33 -0700 Subject: [PATCH] Alternative fix to GenerateChangePhoneNumber --- .../UserManagerSpecificationTests.cs | 41 ++++++++++++++++++- .../TokenOptions.cs | 2 +- .../UserManager.cs | 23 ++++------- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs b/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs index dd9c4c427b..d960d3f6a9 100644 --- a/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs +++ b/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs @@ -1568,12 +1568,48 @@ namespace Microsoft.AspNetCore.Identity.Test var stamp = await manager.GetSecurityStampAsync(user); IdentityResultAssert.IsFailure(await manager.ChangePhoneNumberAsync(user, "111-111-1111", "bogus"), "Invalid token."); - IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyChangePhoneNumberTokenAsync() failed for user { await manager.GetUserIdAsync(user)}."); + IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyUserTokenAsync() failed with purpose: ChangePhoneNumber:111-111-1111 for user {await manager.GetUserIdAsync(user)}."); Assert.False(await manager.IsPhoneNumberConfirmedAsync(user)); Assert.Equal("123-456-7890", await manager.GetPhoneNumberAsync(user)); Assert.Equal(stamp, await manager.GetSecurityStampAsync(user)); } + private class YesPhoneNumberProvider : IUserTwoFactorTokenProvider + { + public Task CanGenerateTwoFactorTokenAsync(UserManager manager, TUser user) + => Task.FromResult(true); + + public Task GenerateAsync(string purpose, UserManager manager, TUser user) + => Task.FromResult(purpose); + + public Task ValidateAsync(string purpose, string token, UserManager manager, TUser user) + => Task.FromResult(true); + } + + /// + /// Test. + /// + /// Task + [Fact] + public async Task ChangePhoneNumberWithCustomProvider() + { + if (ShouldSkipDbTests()) + { + return; + } + var manager = CreateManager(); + manager.RegisterTokenProvider("Yes", new YesPhoneNumberProvider()); + manager.Options.Tokens.ChangePhoneNumberTokenProvider = "Yes"; + var user = CreateTestUser(phoneNumber: "123-456-7890"); + IdentityResultAssert.IsSuccess(await manager.CreateAsync(user)); + Assert.False(await manager.IsPhoneNumberConfirmedAsync(user)); + var stamp = await manager.GetSecurityStampAsync(user); + IdentityResultAssert.IsSuccess(await manager.ChangePhoneNumberAsync(user, "111-111-1111", "whatever")); + Assert.True(await manager.IsPhoneNumberConfirmedAsync(user)); + Assert.Equal("111-111-1111", await manager.GetPhoneNumberAsync(user)); + Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user)); + } + /// /// Test. /// @@ -1623,7 +1659,8 @@ namespace Microsoft.AspNetCore.Identity.Test Assert.True(await manager.VerifyChangePhoneNumberTokenAsync(user, token2, num2)); Assert.False(await manager.VerifyChangePhoneNumberTokenAsync(user, token2, num1)); Assert.False(await manager.VerifyChangePhoneNumberTokenAsync(user, token1, num2)); - IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyChangePhoneNumberTokenAsync() failed for user {await manager.GetUserIdAsync(user)}."); + IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyUserTokenAsync() failed with purpose: ChangePhoneNumber:{num1} for user {await manager.GetUserIdAsync(user)}."); + IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyUserTokenAsync() failed with purpose: ChangePhoneNumber:{num2} for user {await manager.GetUserIdAsync(user)}."); } /// diff --git a/src/Microsoft.Extensions.Identity.Core/TokenOptions.cs b/src/Microsoft.Extensions.Identity.Core/TokenOptions.cs index bb54619c2e..8c969769a1 100644 --- a/src/Microsoft.Extensions.Identity.Core/TokenOptions.cs +++ b/src/Microsoft.Extensions.Identity.Core/TokenOptions.cs @@ -66,7 +66,7 @@ namespace Microsoft.AspNetCore.Identity /// /// The used to generate tokens used when changing phone numbers. /// - public string ChangePhoneNumberTokenProvider { get; set; } = DefaultProvider; + public string ChangePhoneNumberTokenProvider { get; set; } = DefaultPhoneProvider; /// /// Gets or sets the used to validate two factor sign ins with an authenticator. diff --git a/src/Microsoft.Extensions.Identity.Core/UserManager.cs b/src/Microsoft.Extensions.Identity.Core/UserManager.cs index 7956c06da5..2ee45b8431 100644 --- a/src/Microsoft.Extensions.Identity.Core/UserManager.cs +++ b/src/Microsoft.Extensions.Identity.Core/UserManager.cs @@ -1579,12 +1579,10 @@ namespace Microsoft.AspNetCore.Identity /// /// The that represents the asynchronous operation, containing the telephone change number token. /// - public virtual async Task GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber) + public virtual Task GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber) { ThrowIfDisposed(); - return Rfc6238AuthenticationService.GenerateCode( - await CreateSecurityTokenAsync(user), phoneNumber) - .ToString(CultureInfo.InvariantCulture); + return GenerateUserTokenAsync(user, Options.Tokens.ChangePhoneNumberTokenProvider, ChangePhoneNumberTokenPurpose + ":" + phoneNumber); } /// @@ -1598,21 +1596,16 @@ namespace Microsoft.AspNetCore.Identity /// The that represents the asynchronous operation, returning true if the /// is valid, otherwise false. /// - public virtual async Task VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber) + public virtual Task VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber) { ThrowIfDisposed(); - - var securityToken = await CreateSecurityTokenAsync(user); - int code; - if (securityToken != null && Int32.TryParse(token, out code)) + if (user == null) { - if (Rfc6238AuthenticationService.ValidateCode(securityToken, code, phoneNumber)) - { - return true; - } + throw new ArgumentNullException(nameof(user)); } - Logger.LogWarning(8, "VerifyChangePhoneNumberTokenAsync() failed for user {userId}.", await GetUserIdAsync(user)); - return false; + + // Make sure the token is valid and the stamp matches + return VerifyUserTokenAsync(user, Options.Tokens.ChangePhoneNumberTokenProvider, ChangePhoneNumberTokenPurpose+":"+ phoneNumber, token); } ///