Fix regression with ChangePhoneNumber tokens (#1392)

aspnet · Aug 24, 2017 · e36e681 · e36e681
1 parent 0f5bc9d
commit e36e681
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 10 deletions.
diff --git a/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs b/src/Microsoft.AspNetCore.Identity.Specification.Tests/UserManagerSpecificationTests.cs
@@ -1532,6 +1532,24 @@ public async Task CanChangePhoneNumber()
             Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user));
         }
 
+        /// <summary>
+        /// Test.
+        /// </summary>
+        /// <returns>Task</returns>
+        [Fact]
+        public async Task ChangePhoneNumberTokenIsInt()
+        {
+            if (ShouldSkipDbTests())
+            {
+                return;
+            }
+            var manager = CreateManager();
+            var user = CreateTestUser(phoneNumber: "123-456-7890");
+            IdentityResultAssert.IsSuccess(await manager.CreateAsync(user));
+            var token1 = await manager.GenerateChangePhoneNumberTokenAsync(user, "111-111-1111");
+            Assert.True(int.TryParse(token1, out var ignored));
+        }
+
         /// <summary>
         /// Test.
         /// </summary>
@@ -1550,7 +1568,7 @@ public async Task ChangePhoneNumberFailsWithWrongToken()
             var stamp = await manager.GetSecurityStampAsync(user);
             IdentityResultAssert.IsFailure(await manager.ChangePhoneNumberAsync(user, "111-111-1111", "bogus"),
                 "Invalid token.");
-            IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyUserTokenAsync() failed with purpose: ChangePhoneNumber:111-111-1111 for user { await manager.GetUserIdAsync(user)}.");
+            IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyChangePhoneNumberTokenAsync() failed 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));
@@ -1605,7 +1623,7 @@ public async Task CanVerifyPhoneNumber()
             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, $"VerifyUserTokenAsync() failed with purpose: ChangePhoneNumber:111-123-4567 for user {await manager.GetUserIdAsync(user)}.");
+            IdentityResultAssert.VerifyLogMessage(manager.Logger, $"VerifyChangePhoneNumberTokenAsync() failed for user {await manager.GetUserIdAsync(user)}.");
         }
 
         /// <summary>

diff --git a/src/Microsoft.Extensions.Identity.Core/UserManager.cs b/src/Microsoft.Extensions.Identity.Core/UserManager.cs
@@ -1579,10 +1579,12 @@ public virtual Task<bool> IsPhoneNumberConfirmedAsync(TUser user)
         /// <returns>
         /// The <see cref="Task"/> that represents the asynchronous operation, containing the telephone change number token.
         /// </returns>
-        public virtual Task<string> GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber)
+        public virtual async Task<string> GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber)
         {
             ThrowIfDisposed();
-            return GenerateUserTokenAsync(user, Options.Tokens.ChangePhoneNumberTokenProvider, ChangePhoneNumberTokenPurpose + ":" + phoneNumber);
+            return Rfc6238AuthenticationService.GenerateCode(
+                await CreateSecurityTokenAsync(user), phoneNumber)
+                    .ToString(CultureInfo.InvariantCulture);
         }
 
         /// <summary>
@@ -1596,16 +1598,21 @@ public virtual Task<string> GenerateChangePhoneNumberTokenAsync(TUser user, stri
         /// The <see cref="Task"/> that represents the asynchronous operation, returning true if the <paramref name="token"/>
         /// is valid, otherwise false.
         /// </returns>
-        public virtual Task<bool> VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber)
+        public virtual async Task<bool> VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber)
         {
             ThrowIfDisposed();
-            if (user == null)
+
+            var securityToken = await CreateSecurityTokenAsync(user);
+            int code;
+            if (securityToken != null && Int32.TryParse(token, out code))
             {
-                throw new ArgumentNullException(nameof(user));
+                if (Rfc6238AuthenticationService.ValidateCode(securityToken, code, phoneNumber))
+                {
+                    return true;
+                }
             }
-
-            // Make sure the token is valid and the stamp matches
-            return VerifyUserTokenAsync(user, Options.Tokens.ChangePhoneNumberTokenProvider, ChangePhoneNumberTokenPurpose+":"+ phoneNumber, token);
+            Logger.LogWarning(8, "VerifyChangePhoneNumberTokenAsync() failed for user {userId}.", await GetUserIdAsync(user));
+            return false;
         }
 
         /// <summary>