From c6a82ad19a0bdb80b7358dc843b66664bbf33f96 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 6 Feb 2018 13:27:53 -0800 Subject: [PATCH] TFA cookies now validate security stamp (#1351) --- src/Core/UserManager.cs | 1 + .../ITwoFactorSecurityStampValidator.cs | 12 ++ .../IdentityServiceCollectionExtensions.cs | 17 ++- src/Identity/SecurityStampValidator.cs | 108 ++++++++++---- src/Identity/SignInManager.cs | 61 ++++++-- .../TwoFactorSecurityStampValidator.cs | 44 ++++++ .../UserManagerSpecificationTests.cs | 22 ++- .../Manage/ResetAuthenticator.cshtml.cs | 4 + .../Manage/TwoFactorAuthentication.cshtml.cs | 9 +- .../SecurityStampValidatorTest.cs | 141 ++++++++++++------ test/Identity.Test/SignInManagerTest.cs | 3 +- test/InMemory.Test/FunctionalTest.cs | 57 ++++++- 12 files changed, 373 insertions(+), 106 deletions(-) create mode 100644 src/Identity/ITwoFactorSecurityStampValidator.cs create mode 100644 src/Identity/TwoFactorSecurityStampValidator.cs diff --git a/src/Core/UserManager.cs b/src/Core/UserManager.cs index cfe966c0c2..cec8d8eb7e 100644 --- a/src/Core/UserManager.cs +++ b/src/Core/UserManager.cs @@ -2147,6 +2147,7 @@ namespace Microsoft.AspNetCore.Identity throw new ArgumentNullException(nameof(user)); } await store.SetAuthenticatorKeyAsync(user, GenerateNewAuthenticatorKey(), CancellationToken); + await UpdateSecurityStampInternal(user); return await UpdateAsync(user); } diff --git a/src/Identity/ITwoFactorSecurityStampValidator.cs b/src/Identity/ITwoFactorSecurityStampValidator.cs new file mode 100644 index 0000000000..3d624b190d --- /dev/null +++ b/src/Identity/ITwoFactorSecurityStampValidator.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Identity +{ + /// + /// Used to validate the two factor remember client cookie security stamp. + /// + public interface ITwoFactorSecurityStampValidator : ISecurityStampValidator + { } +} + diff --git a/src/Identity/IdentityServiceCollectionExtensions.cs b/src/Identity/IdentityServiceCollectionExtensions.cs index 014b176d5e..d81f1b721b 100644 --- a/src/Identity/IdentityServiceCollectionExtensions.cs +++ b/src/Identity/IdentityServiceCollectionExtensions.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; @@ -69,7 +71,7 @@ namespace Microsoft.Extensions.DependencyInjection options.DefaultChallengeScheme = IdentityConstants.ApplicationScheme; options.DefaultSignInScheme = IdentityConstants.ExternalScheme; }) - .AddCookie(IdentityConstants.ApplicationScheme, o => + .AddCookie(IdentityConstants.ApplicationScheme, o => { o.LoginPath = new PathString("/Account/Login"); o.Events = new CookieAuthenticationEvents @@ -77,13 +79,19 @@ namespace Microsoft.Extensions.DependencyInjection OnValidatePrincipal = SecurityStampValidator.ValidatePrincipalAsync }; }) - .AddCookie(IdentityConstants.ExternalScheme, o => + .AddCookie(IdentityConstants.ExternalScheme, o => { o.Cookie.Name = IdentityConstants.ExternalScheme; o.ExpireTimeSpan = TimeSpan.FromMinutes(5); }) - .AddCookie(IdentityConstants.TwoFactorRememberMeScheme, - o => o.Cookie.Name = IdentityConstants.TwoFactorRememberMeScheme) + .AddCookie(IdentityConstants.TwoFactorRememberMeScheme, o => + { + o.Cookie.Name = IdentityConstants.TwoFactorRememberMeScheme; + o.Events = new CookieAuthenticationEvents + { + OnValidatePrincipal = SecurityStampValidator.ValidateAsync + }; + }) .AddCookie(IdentityConstants.TwoFactorUserIdScheme, o => { o.Cookie.Name = IdentityConstants.TwoFactorUserIdScheme; @@ -101,6 +109,7 @@ namespace Microsoft.Extensions.DependencyInjection // No interface for the error describer so we can add errors without rev'ing the interface services.TryAddScoped(); services.TryAddScoped>(); + services.TryAddScoped>(); services.TryAddScoped, UserClaimsPrincipalFactory>(); services.TryAddScoped, AspNetUserManager>(); services.TryAddScoped, SignInManager>(); diff --git a/src/Identity/SecurityStampValidator.cs b/src/Identity/SecurityStampValidator.cs index 9b79d73108..9a9ecbe139 100644 --- a/src/Identity/SecurityStampValidator.cs +++ b/src/Identity/SecurityStampValidator.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; @@ -16,10 +17,6 @@ namespace Microsoft.AspNetCore.Identity /// The type encapsulating a user. public class SecurityStampValidator : ISecurityStampValidator where TUser : class { - private readonly SignInManager _signInManager; - private readonly SecurityStampValidatorOptions _options; - private ISystemClock _clock; - /// /// Creates a new instance of . /// @@ -36,11 +33,62 @@ namespace Microsoft.AspNetCore.Identity { throw new ArgumentNullException(nameof(signInManager)); } - _signInManager = signInManager; - _options = options.Value; - _clock = clock; + SignInManager = signInManager; + Options = options.Value; + Clock = clock; } + /// + /// The SignInManager. + /// + public SignInManager SignInManager { get; } + + /// + /// The . + /// + public SecurityStampValidatorOptions Options { get; } + + /// + /// The . + /// + public ISystemClock Clock { get; } + + /// + /// Called when the security stamp has been verified. + /// + /// The user who has been verified. + /// The . + /// A task. + protected virtual async Task SecurityStampVerified(TUser user, CookieValidatePrincipalContext context) + { + var newPrincipal = await SignInManager.CreateUserPrincipalAsync(user); + + if (Options.OnRefreshingPrincipal != null) + { + var replaceContext = new SecurityStampRefreshingPrincipalContext + { + CurrentPrincipal = context.Principal, + NewPrincipal = newPrincipal + }; + + // Note: a null principal is allowed and results in a failed authentication. + await Options.OnRefreshingPrincipal(replaceContext); + newPrincipal = replaceContext.NewPrincipal; + } + + // REVIEW: note we lost login authentication method + context.ReplacePrincipal(newPrincipal); + context.ShouldRenew = true; + } + + /// + /// Verifies the principal's security stamp, returns the matching user if successful + /// + /// The principal to verify. + /// The verified user or null if verification fails. + protected virtual Task VerifySecurityStamp(ClaimsPrincipal principal) + => SignInManager.ValidateSecurityStampAsync(principal); + /// /// Validates a security stamp of an identity as an asynchronous operation, and rebuilds the identity if the validation succeeds, otherwise rejects /// the identity. @@ -51,9 +99,9 @@ namespace Microsoft.AspNetCore.Identity public virtual async Task ValidateAsync(CookieValidatePrincipalContext context) { var currentUtc = DateTimeOffset.UtcNow; - if (context.Options != null && _clock != null) + if (context.Options != null && Clock != null) { - currentUtc = _clock.UtcNow; + currentUtc = Clock.UtcNow; } var issuedUtc = context.Properties.IssuedUtc; @@ -62,36 +110,19 @@ namespace Microsoft.AspNetCore.Identity if (issuedUtc != null) { var timeElapsed = currentUtc.Subtract(issuedUtc.Value); - validate = timeElapsed > _options.ValidationInterval; + validate = timeElapsed > Options.ValidationInterval; } if (validate) { - var user = await _signInManager.ValidateSecurityStampAsync(context.Principal); + var user = await VerifySecurityStamp(context.Principal); if (user != null) { - var newPrincipal = await _signInManager.CreateUserPrincipalAsync(user); - - if (_options.OnRefreshingPrincipal != null) - { - var replaceContext = new SecurityStampRefreshingPrincipalContext - { - CurrentPrincipal = context.Principal, - NewPrincipal = newPrincipal - }; - - // Note: a null principal is allowed and results in a failed authentication. - await _options.OnRefreshingPrincipal(replaceContext); - newPrincipal = replaceContext.NewPrincipal; - } - - // REVIEW: note we lost login authentication method - context.ReplacePrincipal(newPrincipal); - context.ShouldRenew = true; + await SecurityStampVerified(user, context); } else { context.RejectPrincipal(); - await _signInManager.SignOutAsync(); + await SignInManager.SignOutAsync(); } } } @@ -105,19 +136,30 @@ namespace Microsoft.AspNetCore.Identity { /// /// Validates a principal against a user's stored security stamp. - /// the identity. /// /// The context containing the - /// and to validate. + /// and to validate. /// The that represents the asynchronous validation operation. public static Task ValidatePrincipalAsync(CookieValidatePrincipalContext context) + => ValidateAsync(context); + + /// + /// Used to validate the and + /// cookies against the user's + /// stored security stamp. + /// + /// The context containing the + /// and to validate. + /// + + public static Task ValidateAsync(CookieValidatePrincipalContext context) where TValidator : ISecurityStampValidator { if (context.HttpContext.RequestServices == null) { throw new InvalidOperationException("RequestServices is null."); } - var validator = context.HttpContext.RequestServices.GetRequiredService(); + var validator = context.HttpContext.RequestServices.GetRequiredService(); return validator.ValidateAsync(context); } } diff --git a/src/Identity/SignInManager.cs b/src/Identity/SignInManager.cs index 608be1af4a..879a38dcf6 100644 --- a/src/Identity/SignInManager.cs +++ b/src/Identity/SignInManager.cs @@ -220,18 +220,46 @@ namespace Microsoft.AspNetCore.Identity return null; } var user = await UserManager.GetUserAsync(principal); - if (user != null && UserManager.SupportsUserSecurityStamp) + if (await ValidateSecurityStampAsync(user, principal.FindFirstValue(Options.ClaimsIdentity.SecurityStampClaimType))) { - var securityStamp = - principal.FindFirstValue(Options.ClaimsIdentity.SecurityStampClaimType); - if (securityStamp == await UserManager.GetSecurityStampAsync(user)) - { - return user; - } + return user; } return null; } + /// + /// Validates the security stamp for the specified from one of + /// the two factor principals (remember client or user id) against + /// the persisted stamp for the current user, as an asynchronous operation. + /// + /// The principal whose stamp should be validated. + /// The task object representing the asynchronous operation. The task will contain the + /// if the stamp matches the persisted value, otherwise it will return false. + public virtual async Task ValidateTwoFactorSecurityStampAsync(ClaimsPrincipal principal) + { + if (principal == null || principal.Identity?.Name == null) + { + return null; + } + var user = await UserManager.FindByIdAsync(principal.Identity.Name); + if (await ValidateSecurityStampAsync(user, principal.FindFirstValue(Options.ClaimsIdentity.SecurityStampClaimType))) + { + return user; + } + return null; + } + + /// + /// Validates the security stamp for the specified . Will always return false + /// if the userManager does not support security stamps. + /// + /// The user whose stamp should be validated. + /// The expected security stamp value. + /// True if the stamp matches the persisted value, otherwise it will return false. + public virtual async Task ValidateSecurityStampAsync(TUser user, string securityStamp) + => user != null && UserManager.SupportsUserSecurityStamp + && securityStamp == await UserManager.GetSecurityStampAsync(user); + /// /// Attempts to sign in the specified and combination /// as an asynchronous operation. @@ -343,11 +371,9 @@ namespace Microsoft.AspNetCore.Identity /// The task object representing the asynchronous operation. public virtual async Task RememberTwoFactorClientAsync(TUser user) { - var userId = await UserManager.GetUserIdAsync(user); - var rememberBrowserIdentity = new ClaimsIdentity(IdentityConstants.TwoFactorRememberMeScheme); - rememberBrowserIdentity.AddClaim(new Claim(ClaimTypes.Name, userId)); + var principal = await StoreRememberClient(user); await Context.SignInAsync(IdentityConstants.TwoFactorRememberMeScheme, - new ClaimsPrincipal(rememberBrowserIdentity), + principal, new AuthenticationProperties { IsPersistent = true }); } @@ -655,6 +681,19 @@ namespace Microsoft.AspNetCore.Identity return new ClaimsPrincipal(identity); } + internal async Task StoreRememberClient(TUser user) + { + var userId = await UserManager.GetUserIdAsync(user); + var rememberBrowserIdentity = new ClaimsIdentity(IdentityConstants.TwoFactorRememberMeScheme); + rememberBrowserIdentity.AddClaim(new Claim(ClaimTypes.Name, userId)); + if (UserManager.SupportsUserSecurityStamp) + { + var stamp = await UserManager.GetSecurityStampAsync(user); + rememberBrowserIdentity.AddClaim(new Claim(Options.ClaimsIdentity.SecurityStampClaimType, stamp)); + } + return new ClaimsPrincipal(rememberBrowserIdentity); + } + private ClaimsIdentity CreateIdentity(TwoFactorAuthenticationInfo info) { if (info == null) diff --git a/src/Identity/TwoFactorSecurityStampValidator.cs b/src/Identity/TwoFactorSecurityStampValidator.cs new file mode 100644 index 0000000000..7d0feb7d39 --- /dev/null +++ b/src/Identity/TwoFactorSecurityStampValidator.cs @@ -0,0 +1,44 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Identity +{ + /// + /// Responsible for validation of two factor identity cookie security stamp. + /// + /// The type encapsulating a user. + public class TwoFactorSecurityStampValidator : SecurityStampValidator, ITwoFactorSecurityStampValidator where TUser : class + { + /// + /// Creates a new instance of . + /// + /// Used to access the . + /// The . + /// The system clock. + public TwoFactorSecurityStampValidator(IOptions options, SignInManager signInManager, ISystemClock clock) : base(options, signInManager, clock) + { } + + /// + /// Verifies the principal's security stamp, returns the matching user if successful + /// + /// The principal to verify. + /// The verified user or null if verification fails. + protected override Task VerifySecurityStamp(ClaimsPrincipal principal) + => SignInManager.ValidateTwoFactorSecurityStampAsync(principal); + + /// + /// Called when the security stamp has been verified. + /// + /// The user who has been verified. + /// The . + /// A task. + protected override Task SecurityStampVerified(TUser user, CookieValidatePrincipalContext context) + => Task.CompletedTask; + } +} \ No newline at end of file diff --git a/src/Specification.Tests/UserManagerSpecificationTests.cs b/src/Specification.Tests/UserManagerSpecificationTests.cs index ab39df6aff..99c4049c05 100644 --- a/src/Specification.Tests/UserManagerSpecificationTests.cs +++ b/src/Specification.Tests/UserManagerSpecificationTests.cs @@ -290,7 +290,27 @@ namespace Microsoft.AspNetCore.Identity.Test var user = CreateTestUser(username, useNamePrefixAsUserName: true); var stamp = await manager.GetSecurityStampAsync(user); IdentityResultAssert.IsSuccess(await manager.CreateAsync(user)); - Assert.NotNull(await manager.GetSecurityStampAsync(user)); + Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user)); + } + + /// + /// Test. + /// + /// Task + [Fact] + public async Task ResetAuthenticatorKeyUpdatesSecurityStamp() + { + if (ShouldSkipDbTests()) + { + return; + } + var manager = CreateManager(); + var username = "Create" + Guid.NewGuid().ToString(); + var user = CreateTestUser(username, useNamePrefixAsUserName: true); + IdentityResultAssert.IsSuccess(await manager.CreateAsync(user)); + var stamp = await manager.GetSecurityStampAsync(user); + IdentityResultAssert.IsSuccess(await manager.ResetAuthenticatorKeyAsync(user)); + Assert.NotEqual(stamp, await manager.GetSecurityStampAsync(user)); } /// diff --git a/src/UI/Areas/Identity/Pages/Account/Manage/ResetAuthenticator.cshtml.cs b/src/UI/Areas/Identity/Pages/Account/Manage/ResetAuthenticator.cshtml.cs index 7545bdefa1..f8fb0da2cf 100644 --- a/src/UI/Areas/Identity/Pages/Account/Manage/ResetAuthenticator.cshtml.cs +++ b/src/UI/Areas/Identity/Pages/Account/Manage/ResetAuthenticator.cshtml.cs @@ -23,13 +23,16 @@ namespace Microsoft.AspNetCore.Identity.UI.Pages.Account.Manage.Internal internal class ResetAuthenticatorModel : ResetAuthenticatorModel where TUser : IdentityUser { UserManager _userManager; + private readonly SignInManager _signInManager; ILogger _logger; public ResetAuthenticatorModel( UserManager userManager, + SignInManager signInManager, ILogger logger) { _userManager = userManager; + _signInManager = signInManager; _logger = logger; } @@ -56,6 +59,7 @@ namespace Microsoft.AspNetCore.Identity.UI.Pages.Account.Manage.Internal await _userManager.ResetAuthenticatorKeyAsync(user); _logger.LogInformation("User with ID '{UserId}' has reset their authentication app key.", user.Id); + await _signInManager.SignInAsync(user, isPersistent: false); StatusMessage = "Your authenticator app key has been reset, you will need to configure your authenticator app using the new key."; return RedirectToPage("./EnableAuthenticator"); diff --git a/src/UI/Areas/Identity/Pages/Account/Manage/TwoFactorAuthentication.cshtml.cs b/src/UI/Areas/Identity/Pages/Account/Manage/TwoFactorAuthentication.cshtml.cs index a650e16f2c..08cc7283e4 100644 --- a/src/UI/Areas/Identity/Pages/Account/Manage/TwoFactorAuthentication.cshtml.cs +++ b/src/UI/Areas/Identity/Pages/Account/Manage/TwoFactorAuthentication.cshtml.cs @@ -24,7 +24,10 @@ namespace Microsoft.AspNetCore.Identity.UI.Pages.Account.Manage.Internal [TempData] public string StatusMessage { get; set; } - public virtual Task OnGet() => throw new NotImplementedException(); + public virtual Task OnGetAsync() => throw new NotImplementedException(); + + public virtual Task OnPostAsync() => throw new NotImplementedException(); + } internal class TwoFactorAuthenticationModel : TwoFactorAuthenticationModel where TUser : IdentityUser @@ -41,7 +44,7 @@ namespace Microsoft.AspNetCore.Identity.UI.Pages.Account.Manage.Internal _logger = logger; } - public override async Task OnGet() + public override async Task OnGetAsync() { var user = await _userManager.GetUserAsync(User); if (user == null) @@ -57,7 +60,7 @@ namespace Microsoft.AspNetCore.Identity.UI.Pages.Account.Manage.Internal return Page(); } - public async Task OnPost() + public override async Task OnPostAsync() { var user = await _userManager.GetUserAsync(User); if (user == null) diff --git a/test/Identity.Test/SecurityStampValidatorTest.cs b/test/Identity.Test/SecurityStampValidatorTest.cs index cbbd9617c0..5b6b76a9e3 100644 --- a/test/Identity.Test/SecurityStampValidatorTest.cs +++ b/test/Identity.Test/SecurityStampValidatorTest.cs @@ -72,40 +72,56 @@ namespace Microsoft.AspNetCore.Identity.Test public async Task OnValidatePrincipalTestSuccess(bool isPersistent) { var user = new TestUser("test"); + var httpContext = new Mock(); + + await RunApplicationCookieTest(user, httpContext, /*shouldStampValidate*/true, async () => + { + var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme); + id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); + var principal = new ClaimsPrincipal(id); + var properties = new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow.AddSeconds(-1), IsPersistent = isPersistent }; + var ticket = new AuthenticationTicket(principal, + properties, + IdentityConstants.ApplicationScheme); + + var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(), new CookieAuthenticationOptions(), ticket); + Assert.NotNull(context.Properties); + Assert.NotNull(context.Options); + Assert.NotNull(context.Principal); + await SecurityStampValidator.ValidatePrincipalAsync(context); + Assert.NotNull(context.Principal); + }); + } + + private async Task RunApplicationCookieTest(TestUser user, Mock httpContext, bool shouldStampValidate, Func testCode) + { var userManager = MockHelpers.MockUserManager(); var claimsManager = new Mock>(); var identityOptions = new Mock>(); identityOptions.Setup(a => a.Value).Returns(new IdentityOptions()); var options = new Mock>(); options.Setup(a => a.Value).Returns(new SecurityStampValidatorOptions { ValidationInterval = TimeSpan.Zero }); - var httpContext = new Mock(); var contextAccessor = new Mock(); contextAccessor.Setup(a => a.HttpContext).Returns(httpContext.Object); - var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme); - id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); - var principal = new ClaimsPrincipal(id); - - var properties = new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow.AddSeconds(-1), IsPersistent = isPersistent }; var signInManager = new Mock>(userManager.Object, contextAccessor.Object, claimsManager.Object, identityOptions.Object, null, new Mock().Object); - signInManager.Setup(s => s.ValidateSecurityStampAsync(It.IsAny())).ReturnsAsync(user).Verifiable(); - signInManager.Setup(s => s.CreateUserPrincipalAsync(user)).ReturnsAsync(principal).Verifiable(); + signInManager.Setup(s => s.ValidateSecurityStampAsync(It.IsAny())).ReturnsAsync(shouldStampValidate ? user : default(TestUser)).Verifiable(); + + if (shouldStampValidate) + { + var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme); + id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); + var principal = new ClaimsPrincipal(id); + signInManager.Setup(s => s.CreateUserPrincipalAsync(user)).ReturnsAsync(principal).Verifiable(); + } + var services = new ServiceCollection(); services.AddSingleton(options.Object); services.AddSingleton(signInManager.Object); services.AddSingleton(new SecurityStampValidator(options.Object, signInManager.Object, new SystemClock())); httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider()); - var ticket = new AuthenticationTicket(principal, - properties, - IdentityConstants.ApplicationScheme); - var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(), new CookieAuthenticationOptions(), ticket); - Assert.NotNull(context.Properties); - Assert.NotNull(context.Options); - Assert.NotNull(context.Principal); - await - SecurityStampValidator.ValidatePrincipalAsync(context); - Assert.NotNull(context.Principal); + await testCode.Invoke(); signInManager.VerifyAll(); } @@ -113,36 +129,23 @@ namespace Microsoft.AspNetCore.Identity.Test public async Task OnValidateIdentityRejectsWhenValidateSecurityStampFails() { var user = new TestUser("test"); - var userManager = MockHelpers.MockUserManager(); - var claimsManager = new Mock>(); - var identityOptions = new Mock>(); - identityOptions.Setup(a => a.Value).Returns(new IdentityOptions()); - var options = new Mock>(); - options.Setup(a => a.Value).Returns(new SecurityStampValidatorOptions { ValidationInterval = TimeSpan.Zero }); var httpContext = new Mock(); - var contextAccessor = new Mock(); - contextAccessor.Setup(a => a.HttpContext).Returns(httpContext.Object); - var signInManager = new Mock>(userManager.Object, - contextAccessor.Object, claimsManager.Object, identityOptions.Object, null, new Mock().Object); - signInManager.Setup(s => s.ValidateSecurityStampAsync(It.IsAny())).ReturnsAsync(default(TestUser)).Verifiable(); - var services = new ServiceCollection(); - services.AddSingleton(options.Object); - services.AddSingleton(signInManager.Object); - services.AddSingleton(new SecurityStampValidator(options.Object, signInManager.Object, new SystemClock())); - httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider()); - var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme); - id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); - var ticket = new AuthenticationTicket(new ClaimsPrincipal(id), - new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow.AddSeconds(-1) }, - IdentityConstants.ApplicationScheme); - var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(), new CookieAuthenticationOptions(), ticket); - Assert.NotNull(context.Properties); - Assert.NotNull(context.Options); - Assert.NotNull(context.Principal); - await SecurityStampValidator.ValidatePrincipalAsync(context); - Assert.Null(context.Principal); - signInManager.VerifyAll(); + await RunApplicationCookieTest(user, httpContext, /*shouldStampValidate*/false, async () => + { + var id = new ClaimsIdentity(IdentityConstants.ApplicationScheme); + id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); + var ticket = new AuthenticationTicket(new ClaimsPrincipal(id), + new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow.AddSeconds(-1) }, + IdentityConstants.ApplicationScheme); + + var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(), new CookieAuthenticationOptions(), ticket); + Assert.NotNull(context.Properties); + Assert.NotNull(context.Options); + Assert.NotNull(context.Principal); + await SecurityStampValidator.ValidatePrincipalAsync(context); + Assert.Null(context.Principal); + }); } [Fact] @@ -216,5 +219,51 @@ namespace Microsoft.AspNetCore.Identity.Test await SecurityStampValidator.ValidatePrincipalAsync(context); Assert.NotNull(context.Principal); } + + private async Task RunRememberClientCookieTest(bool shouldStampValidate, bool validationSuccess) + { + var user = new TestUser("test"); + var httpContext = new Mock(); + var userManager = MockHelpers.MockUserManager(); + userManager.Setup(u => u.GetUserIdAsync(user)).ReturnsAsync(user.Id).Verifiable(); + var claimsManager = new Mock>(); + var identityOptions = new Mock>(); + identityOptions.Setup(a => a.Value).Returns(new IdentityOptions()); + var options = new Mock>(); + options.Setup(a => a.Value).Returns(new SecurityStampValidatorOptions { ValidationInterval = TimeSpan.Zero }); + var contextAccessor = new Mock(); + contextAccessor.Setup(a => a.HttpContext).Returns(httpContext.Object); + var signInManager = new Mock>(userManager.Object, + contextAccessor.Object, claimsManager.Object, identityOptions.Object, null, new Mock().Object); + signInManager.Setup(s => s.ValidateTwoFactorSecurityStampAsync(It.IsAny())).ReturnsAsync(shouldStampValidate ? user : default).Verifiable(); + + var services = new ServiceCollection(); + services.AddSingleton(options.Object); + services.AddSingleton(signInManager.Object); + services.AddSingleton(new TwoFactorSecurityStampValidator(options.Object, signInManager.Object, new SystemClock())); + httpContext.Setup(c => c.RequestServices).Returns(services.BuildServiceProvider()); + + var principal = await signInManager.Object.StoreRememberClient(user); + var ticket = new AuthenticationTicket(principal, + new AuthenticationProperties { IsPersistent = true }, + IdentityConstants.TwoFactorRememberMeScheme); + var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(), new CookieAuthenticationOptions(), ticket); + Assert.NotNull(context.Properties); + Assert.NotNull(context.Options); + Assert.NotNull(context.Principal); + await SecurityStampValidator.ValidateAsync(context); + Assert.Equal(validationSuccess, context.Principal != null); + + signInManager.VerifyAll(); + userManager.VerifyAll(); + } + + [Fact] + public Task TwoFactorRememberClientOnValidatePrincipalTestSuccess() + => RunRememberClientCookieTest(shouldStampValidate: true, validationSuccess: true); + + [Fact] + public Task TwoFactorRememberClientOnValidatePrincipalRejectsWhenValidateSecurityStampFails() + => RunRememberClientCookieTest(shouldStampValidate: false, validationSuccess: false); } } \ No newline at end of file diff --git a/test/Identity.Test/SignInManagerTest.cs b/test/Identity.Test/SignInManagerTest.cs index cfec3620cb..05709478e6 100644 --- a/test/Identity.Test/SignInManagerTest.cs +++ b/test/Identity.Test/SignInManagerTest.cs @@ -8,7 +8,6 @@ using System.Security.Claims; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -17,7 +16,7 @@ using Xunit; namespace Microsoft.AspNetCore.Identity.Test { - public class SignManagerInTest + public class SignInManagerTest { //[Theory] //[InlineData(true)] diff --git a/test/InMemory.Test/FunctionalTest.cs b/test/InMemory.Test/FunctionalTest.cs index 6be648c38d..28b774cd52 100644 --- a/test/InMemory.Test/FunctionalTest.cs +++ b/test/InMemory.Test/FunctionalTest.cs @@ -184,7 +184,8 @@ namespace Microsoft.AspNetCore.Identity.InMemory [Fact] public async Task TwoFactorRememberCookieVerification() { - var server = CreateServer(); + var clock = new TestClock(); + var server = CreateServer(services => services.AddSingleton(clock)); var transaction1 = await SendAsync(server, "http://example.com/createMe"); Assert.Equal(HttpStatusCode.OK, transaction1.Response.StatusCode); @@ -199,6 +200,46 @@ namespace Microsoft.AspNetCore.Identity.InMemory var transaction3 = await SendAsync(server, "http://example.com/isTwoFactorRememebered", transaction2.CookieNameValue); Assert.Equal(HttpStatusCode.OK, transaction3.Response.StatusCode); + + // Wait for validation interval + clock.Add(TimeSpan.FromMinutes(30)); + + var transaction4 = await SendAsync(server, "http://example.com/isTwoFactorRememebered", transaction2.CookieNameValue); + Assert.Equal(HttpStatusCode.OK, transaction4.Response.StatusCode); + } + + [Fact] + public async Task TwoFactorRememberCookieClearedBySecurityStampChange() + { + var clock = new TestClock(); + var server = CreateServer(services => services.AddSingleton(clock)); + + var transaction1 = await SendAsync(server, "http://example.com/createMe"); + Assert.Equal(HttpStatusCode.OK, transaction1.Response.StatusCode); + Assert.Null(transaction1.SetCookie); + + var transaction2 = await SendAsync(server, "http://example.com/twofactorRememeber"); + Assert.Equal(HttpStatusCode.OK, transaction2.Response.StatusCode); + + var setCookie = transaction2.SetCookie; + Assert.Contains(IdentityConstants.TwoFactorRememberMeScheme + "=", setCookie); + Assert.Contains("; expires=", setCookie); + + var transaction3 = await SendAsync(server, "http://example.com/isTwoFactorRememebered", transaction2.CookieNameValue); + Assert.Equal(HttpStatusCode.OK, transaction3.Response.StatusCode); + + var transaction4 = await SendAsync(server, "http://example.com/signoutEverywhere", transaction2.CookieNameValue); + Assert.Equal(HttpStatusCode.OK, transaction4.Response.StatusCode); + + // Doesn't validate until after interval has passed + var transaction5 = await SendAsync(server, "http://example.com/isTwoFactorRememebered", transaction2.CookieNameValue); + Assert.Equal(HttpStatusCode.OK, transaction5.Response.StatusCode); + + // Wait for validation interval + clock.Add(TimeSpan.FromMinutes(30)); + + var transaction6 = await SendAsync(server, "http://example.com/isTwoFactorRememebered", transaction2.CookieNameValue); + Assert.Equal(HttpStatusCode.InternalServerError, transaction6.Response.StatusCode); } private static string FindClaimValue(Transaction transaction, string claimType) @@ -249,9 +290,11 @@ namespace Microsoft.AspNetCore.Identity.InMemory var result = await userManager.CreateAsync(new TestUser("simple"), "aaaaaa"); res.StatusCode = result.Succeeded ? 200 : 500; } - else if (req.Path == new PathString("/protected")) + else if (req.Path == new PathString("/signoutEverywhere")) { - res.StatusCode = 401; + var user = await userManager.FindByNameAsync("hao"); + var result = await userManager.UpdateSecurityStampAsync(user); + res.StatusCode = result.Succeeded ? 200 : 500; } else if (req.Path.StartsWithSegments(new PathString("/pwdLogin"), out remainder)) { @@ -271,8 +314,10 @@ namespace Microsoft.AspNetCore.Identity.InMemory var result = await signInManager.IsTwoFactorClientRememberedAsync(user); res.StatusCode = result ? 200 : 500; } - else if (req.Path == new PathString("/twofactorSignIn")) + else if (req.Path == new PathString("/hasTwoFactorUserId")) { + var result = await context.AuthenticateAsync(IdentityConstants.TwoFactorUserIdScheme); + res.StatusCode = result.Succeeded ? 200 : 500; } else if (req.Path == new PathString("/me")) { @@ -295,7 +340,7 @@ namespace Microsoft.AspNetCore.Identity.InMemory }) .ConfigureServices(services => { - services.AddIdentity(); + services.AddIdentity().AddDefaultTokenProviders(); services.AddSingleton, InMemoryStore>(); services.AddSingleton, InMemoryStore>(); configureServices?.Invoke(services); @@ -346,7 +391,7 @@ namespace Microsoft.AspNetCore.Identity.InMemory }; if (transaction.Response.Headers.Contains("Set-Cookie")) { - transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").SingleOrDefault(); + transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").FirstOrDefault(); } if (!string.IsNullOrEmpty(transaction.SetCookie)) {