[Identity] Guard against null user security stamps (#8300)

Also fix up some UI issues

src/Identity/Core/ref/Microsoft.AspNetCore.Identity.netcoreapp3.0.cs

@@ -94,8 +94,9 @@ namespace Microsoft.AspNetCore.Identity
     }
     public partial class SecurityStampValidator<TUser> : Microsoft.AspNetCore.Identity.ISecurityStampValidator where TUser : class
     {
-        public SecurityStampValidator(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions> options, Microsoft.AspNetCore.Identity.SignInManager<TUser> signInManager, Microsoft.AspNetCore.Authentication.ISystemClock clock) { }
+        public SecurityStampValidator(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions> options, Microsoft.AspNetCore.Identity.SignInManager<TUser> signInManager, Microsoft.AspNetCore.Authentication.ISystemClock clock, Microsoft.Extensions.Logging.ILoggerFactory logger) { }
         public Microsoft.AspNetCore.Authentication.ISystemClock Clock { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
+        public Microsoft.Extensions.Logging.ILogger Logger { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions Options { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         public Microsoft.AspNetCore.Identity.SignInManager<TUser> SignInManager { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         [System.Diagnostics.DebuggerStepThroughAttribute]
@@ -171,7 +172,7 @@ namespace Microsoft.AspNetCore.Identity
     }
     public partial class TwoFactorSecurityStampValidator<TUser> : Microsoft.AspNetCore.Identity.SecurityStampValidator<TUser>, Microsoft.AspNetCore.Identity.ISecurityStampValidator, Microsoft.AspNetCore.Identity.ITwoFactorSecurityStampValidator where TUser : class
     {
-        public TwoFactorSecurityStampValidator(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions> options, Microsoft.AspNetCore.Identity.SignInManager<TUser> signInManager, Microsoft.AspNetCore.Authentication.ISystemClock clock) : base (default(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions>), default(Microsoft.AspNetCore.Identity.SignInManager<TUser>), default(Microsoft.AspNetCore.Authentication.ISystemClock)) { }
+        public TwoFactorSecurityStampValidator(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions> options, Microsoft.AspNetCore.Identity.SignInManager<TUser> signInManager, Microsoft.AspNetCore.Authentication.ISystemClock clock, Microsoft.Extensions.Logging.ILoggerFactory logger) : base (default(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Identity.SecurityStampValidatorOptions>), default(Microsoft.AspNetCore.Identity.SignInManager<TUser>), default(Microsoft.AspNetCore.Authentication.ISystemClock), default(Microsoft.Extensions.Logging.ILoggerFactory)) { }
         protected override System.Threading.Tasks.Task SecurityStampVerified(TUser user, Microsoft.AspNetCore.Authentication.Cookies.CookieValidatePrincipalContext context) { throw null; }
         protected override System.Threading.Tasks.Task<TUser> VerifySecurityStamp(System.Security.Claims.ClaimsPrincipal principal) { throw null; }
     }

src/Identity/Core/src/SecurityStampValidator.cs

@@ -7,6 +7,7 @@ using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Identity
@@ -23,7 +24,8 @@ namespace Microsoft.AspNetCore.Identity
         /// <param name="options">Used to access the <see cref="IdentityOptions"/>.</param>
         /// <param name="signInManager">The <see cref="SignInManager{TUser}"/>.</param>
         /// <param name="clock">The system clock.</param>
-        public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock)
+        /// <param name="logger">The logger.</param>
+        public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock, ILoggerFactory logger)
         {
             if (options == null)
             {
@@ -36,6 +38,7 @@ namespace Microsoft.AspNetCore.Identity
             SignInManager = signInManager;
             Options = options.Value;
             Clock = clock;
+            Logger = logger.CreateLogger(this.GetType().FullName);
         }
 
         /// <summary>
@@ -53,6 +56,14 @@ namespace Microsoft.AspNetCore.Identity
         /// </summary>
         public ISystemClock Clock { get; }
 
+        /// <summary>
+        /// Gets the <see cref="ILogger"/> used to log messages.
+        /// </summary>
+        /// <value>
+        /// The <see cref="ILogger"/> used to log messages.
+        /// </value>
+        public ILogger Logger { get; set; }
+        
         /// <summary>
         /// Called when the security stamp has been verified.
         /// </summary>
@@ -121,6 +132,7 @@ namespace Microsoft.AspNetCore.Identity
                 }
                 else
                 {
+                    Logger.LogDebug(0, "Security stamp validation failed, rejecting cookie.");
                     context.RejectPrincipal();
                     await SignInManager.SignOutAsync();
                 }
@@ -163,4 +175,4 @@ namespace Microsoft.AspNetCore.Identity
             return validator.ValidateAsync(context);
         }
     }
-}
+}

src/Identity/Core/src/SignInManager.cs

@@ -233,6 +233,7 @@ namespace Microsoft.AspNetCore.Identity
             {
                 return user;
             }
+            Logger.LogDebug(4, "Failed to validate a security stamp.");
             return null;
         }
 
@@ -255,6 +256,7 @@ namespace Microsoft.AspNetCore.Identity
             {
                 return user;
             }
+            Logger.LogDebug(5, "Failed to validate a security stamp.");
             return null;
         }
 

src/Identity/Core/src/TwoFactorSecurityStampValidator.cs

@@ -5,6 +5,7 @@ using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Identity
@@ -21,7 +22,8 @@ namespace Microsoft.AspNetCore.Identity
         /// <param name="options">Used to access the <see cref="IdentityOptions"/>.</param>
         /// <param name="signInManager">The <see cref="SignInManager{TUser}"/>.</param>
         /// <param name="clock">The system clock.</param>
-        public TwoFactorSecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock) : base(options, signInManager, clock)
+        /// <param name="logger">The logger.</param>
+        public TwoFactorSecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock, ILoggerFactory logger) : base(options, signInManager, clock, logger)
         { }
 
         /// <summary>
@@ -41,4 +43,4 @@ namespace Microsoft.AspNetCore.Identity
         protected override Task SecurityStampVerified(TUser user, CookieValidatePrincipalContext context)
             => Task.CompletedTask;
     }
-}
+}

src/Identity/EntityFrameworkCore/test/EF.Test/SqlStoreTestBase.cs

@@ -339,6 +339,20 @@ namespace Microsoft.AspNetCore.Identity.EntityFrameworkCore.Test
             Assert.Equal(2, (await manager.GetRolesAsync(userByLogin)).Count);
         }
 
+        [ConditionalFact]
+        [FrameworkSkipCondition(RuntimeFrameworks.Mono)]
+        [OSSkipCondition(OperatingSystems.Linux)]
+        [OSSkipCondition(OperatingSystems.MacOSX)]
+        public async Task GetSecurityStampThrowsIfNull()
+        {
+            var manager = CreateManager();
+            var user = CreateTestUser();
+            var result = await manager.CreateAsync(user);
+            Assert.NotNull(user);
+            user.SecurityStamp = null;
+            await Assert.ThrowsAsync<InvalidOperationException>(async () => await manager.GetSecurityStampAsync(user));
+        }
+
         [ConditionalFact]
         [FrameworkSkipCondition(RuntimeFrameworks.Mono)]
         [OSSkipCondition(OperatingSystems.Linux)]

src/Identity/Extensions.Core/src/UserManager.cs

@@ -860,7 +860,13 @@ namespace Microsoft.AspNetCore.Identity
             {
                 throw new ArgumentNullException(nameof(user));
             }
-            return await securityStore.GetSecurityStampAsync(user, CancellationToken);
+            var stamp = await securityStore.GetSecurityStampAsync(user, CancellationToken);
+            if (stamp == null) 
+            {
+                Logger.LogWarning(15, "GetSecurityStampAsync for user {userId} failed because stamp was null.", await GetUserIdAsync(user));
+                throw new InvalidOperationException(Resources.NullSecurityStamp);
+            }
+            return stamp;
         }
 
         /// <summary>

src/Identity/Extensions.Stores/src/IdentityUser.cs

@@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.Identity
         public IdentityUser()
         {
             Id = Guid.NewGuid().ToString();
+            SecurityStamp = Guid.NewGuid().ToString();
         }
 
         /// <summary>

src/Identity/Specification.Tests/src/UserManagerSpecificationTests.cs

@@ -274,25 +274,6 @@ namespace Microsoft.AspNetCore.Identity.Test
             Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user));
         }
 
-        /// <summary>
-        /// Test.
-        /// </summary>
-        /// <returns>Task</returns>
-        [Fact]
-        public async Task CreateUpdatesSecurityStamp()
-        {
-            if (ShouldSkipDbTests())
-            {
-                return;
-            }
-            var manager = CreateManager();
-            var username = "Create" + Guid.NewGuid().ToString();
-            var user = CreateTestUser(username, useNamePrefixAsUserName: true);
-            var stamp = await manager.GetSecurityStampAsync(user);
-            IdentityResultAssert.IsSuccess(await manager.CreateAsync(user));
-            Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user));
-        }
-
         /// <summary>
         /// Test.
         /// </summary>
@@ -965,10 +946,8 @@ namespace Microsoft.AspNetCore.Identity.Test
             }
             var manager = CreateManager();
             var user = CreateTestUser();
-            Assert.Null(await manager.GetSecurityStampAsync(user));
             IdentityResultAssert.IsSuccess(await manager.CreateAsync(user));
             var stamp = await manager.GetSecurityStampAsync(user);
-            Assert.NotNull(stamp);
             IdentityResultAssert.IsSuccess(await manager.UpdateSecurityStampAsync(user));
             Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user));
         }

src/Identity/UI/src/Areas/Identity/Pages/V3/Account/RegisterConfirmation.cshtml.cs

@@ -4,11 +4,9 @@
 using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Identity.UI.Services;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.RazorPages;
-using Microsoft.Extensions.Hosting;
 
 namespace Microsoft.AspNetCore.Identity.UI.V3.Pages.Account.Internal
 
@@ -49,13 +47,11 @@ namespace Microsoft.AspNetCore.Identity.UI.V3.Pages.Account.Internal
     internal class RegisterConfirmationModel<TUser> : RegisterConfirmationModel where TUser : class
     {
         private readonly UserManager<TUser> _userManager;
-        private readonly IWebHostEnvironment _hostingEnv;
         private readonly IEmailSender _sender;
 
-        public RegisterConfirmationModel(UserManager<TUser> userManager, IWebHostEnvironment hostingEnv, IEmailSender sender)
+        public RegisterConfirmationModel(UserManager<TUser> userManager, IEmailSender sender)
         {
             _userManager = userManager;
-            _hostingEnv = hostingEnv;
             _sender = sender;
         }
 

src/Identity/UI/src/Areas/Identity/Pages/V4/Account/Manage/Email.cshtml

@@ -31,7 +31,7 @@
                 <input asp-for="Input.NewEmail" class="form-control" />
                 <span asp-validation-for="Input.NewEmail" class="text-danger"></span>
             </div>
-            <button id="change-email-button" type="submit" class="btn btn-default">Change email</button>
+            <button id="change-email-button" type="submit" class="btn btn-primary">Change email</button>
         </form>
     </div>
 </div>

src/Identity/UI/src/Areas/Identity/Pages/V4/Account/RegisterConfirmation.cshtml

@@ -4,7 +4,7 @@
     ViewData["Title"] = "Register confirmation";
 }
 
-<h2>@ViewData["Title"]</h2>
+<h1>@ViewData["Title"]</h1>
 @{
     if (@Model.DisplayConfirmAccountLink)
     {

src/Identity/UI/src/Areas/Identity/Pages/V4/Account/RegisterConfirmation.cshtml.cs

@@ -4,11 +4,9 @@
 using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Identity.UI.Services;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.RazorPages;
-using Microsoft.Extensions.Hosting;
 
 namespace Microsoft.AspNetCore.Identity.UI.V4.Pages.Account.Internal
 
@@ -49,13 +47,11 @@ namespace Microsoft.AspNetCore.Identity.UI.V4.Pages.Account.Internal
     internal class RegisterConfirmationModel<TUser> : RegisterConfirmationModel where TUser : class
     {
         private readonly UserManager<TUser> _userManager;
-        private readonly IWebHostEnvironment _hostingEnv;
         private readonly IEmailSender _sender;
 
-        public RegisterConfirmationModel(UserManager<TUser> userManager, IWebHostEnvironment hostingEnv, IEmailSender sender)
+        public RegisterConfirmationModel(UserManager<TUser> userManager, IEmailSender sender)
         {
             _userManager = userManager;
-            _hostingEnv = hostingEnv;
             _sender = sender;
         }
 
@@ -73,7 +69,7 @@ namespace Microsoft.AspNetCore.Identity.UI.V4.Pages.Account.Internal
             }
 
             Email = email;
-            // This sender is a no-op, so we should auto confirm the account
+            // If the email sender is a no-op, display the confirm link in the page
             DisplayConfirmAccountLink = _sender is EmailSender;
             if (DisplayConfirmAccountLink)
             {
@@ -84,7 +80,6 @@ namespace Microsoft.AspNetCore.Identity.UI.V4.Pages.Account.Internal
                     pageHandler: null,
                     values: new { userId = userId, code = code },
                     protocol: Request.Scheme);
-
             }
 
             return Page();

src/Identity/test/Identity.Test/SecurityStampValidatorTest.cs

@@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Moq;
 using Xunit;
@@ -117,7 +118,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             var services = new ServiceCollection();
             services.AddSingleton(options.Object);
             services.AddSingleton(signInManager.Object);
-            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock()));
+            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock(), new LoggerFactory()));
             httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider());
 
             await testCode.Invoke();
@@ -171,7 +172,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             var services = new ServiceCollection();
             services.AddSingleton(options.Object);
             services.AddSingleton(signInManager);
-            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager, new SystemClock()));
+            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager, new SystemClock(), new LoggerFactory()));
             httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider());
 
             var tid = new ClaimsIdentity(IdentityConstants.ApplicationScheme);
@@ -210,7 +211,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             var services = new ServiceCollection();
             services.AddSingleton(options.Object);
             services.AddSingleton(signInManager.Object);
-            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock()));
+            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock(), new LoggerFactory()));
             httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider());
             var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme);
             id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id));
@@ -247,7 +248,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             var services = new ServiceCollection();
             services.AddSingleton(options.Object);
             services.AddSingleton(signInManager.Object);
-            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock()));
+            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock(), new LoggerFactory()));
             httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider());
             var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme);
             id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id));
@@ -283,7 +284,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             var services = new ServiceCollection();
             services.AddSingleton(options.Object);
             services.AddSingleton(signInManager.Object);
-            services.AddSingleton<ITwoFactorSecurityStampValidator>(new TwoFactorSecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock()));
+            services.AddSingleton<ITwoFactorSecurityStampValidator>(new TwoFactorSecurityStampValidator<PocoUser>(options.Object, signInManager.Object, new SystemClock(), new LoggerFactory()));
             httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider());
 
             var principal = await signInManager.Object.StoreRememberClient(user);

src/Identity/test/Identity.Test/UserManagerTest.cs

@@ -84,6 +84,25 @@ namespace Microsoft.AspNetCore.Identity.Test
             store.VerifyAll();
         }
 
+        [Fact]
+        public async Task CreateUpdatesSecurityStampStore()
+        {
+            // Setup
+            var store = new Mock<IUserSecurityStampStore<PocoUser>>();
+            var user = new PocoUser { UserName = "Foo", SecurityStamp = "sssss" };
+            store.Setup(s => s.CreateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable();
+            store.Setup(s => s.GetSecurityStampAsync(user, CancellationToken.None)).Returns(Task.FromResult(user.SecurityStamp)).Verifiable();
+            store.Setup(s => s.SetSecurityStampAsync(user, It.IsAny<string>(), CancellationToken.None)).Returns(Task.FromResult(0)).Verifiable();
+            var userManager = MockHelpers.TestUserManager<PocoUser>(store.Object);
+
+            // Act
+            var result = await userManager.CreateAsync(user);
+
+            // Assert
+            Assert.True(result.Succeeded);
+            store.VerifyAll();
+        }
+
         [Fact]
         public async Task CreateCallsUpdateEmailStore()
         {