From c251bf0162091730b6cc74d97731db74d98e499d Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 23 Apr 2015 22:17:24 -0700 Subject: [PATCH] Add RefreshSignIn --- .../Controllers/ManageController.cs | 17 ++++----- samples/IdentitySample.Mvc/Startup.cs | 5 ++- .../SecurityStampValidator.cs | 8 +--- .../SignInManager.cs | 38 +++++++++++++------ .../Microsoft.AspNet.Identity.Test.xproj | 3 ++ .../SecurityStampValidatorTest.cs | 5 ++- .../SignInManagerTest.cs | 37 +++++++++++++++++- 7 files changed, 81 insertions(+), 32 deletions(-) diff --git a/samples/IdentitySample.Mvc/Controllers/ManageController.cs b/samples/IdentitySample.Mvc/Controllers/ManageController.cs index f33dfe020a..58f53c386a 100644 --- a/samples/IdentitySample.Mvc/Controllers/ManageController.cs +++ b/samples/IdentitySample.Mvc/Controllers/ManageController.cs @@ -72,7 +72,7 @@ namespace IdentitySample.Controllers var result = await UserManager.RemoveLoginAsync(user, loginProvider, providerKey); if (result.Succeeded) { - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); message = ManageMessageId.RemoveLoginSuccess; } } @@ -113,7 +113,7 @@ namespace IdentitySample.Controllers if (user != null) { await SignInManager.RememberTwoFactorClientAsync(user); - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); } return RedirectToAction("Index", "Manage"); } @@ -138,8 +138,7 @@ namespace IdentitySample.Controllers if (user != null) { await UserManager.SetTwoFactorEnabledAsync(user, true); - // TODO: flow remember me somehow? - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); } return RedirectToAction("Index", "Manage"); } @@ -154,7 +153,7 @@ namespace IdentitySample.Controllers if (user != null) { await UserManager.SetTwoFactorEnabledAsync(user, false); - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); } return RedirectToAction("Index", "Manage"); } @@ -187,7 +186,7 @@ namespace IdentitySample.Controllers var result = await UserManager.ChangePhoneNumberAsync(user, model.PhoneNumber, model.Code); if (result.Succeeded) { - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); return RedirectToAction("Index", new { Message = ManageMessageId.AddPhoneSuccess }); } } @@ -207,7 +206,7 @@ namespace IdentitySample.Controllers var result = await UserManager.SetPhoneNumberAsync(user, null); if (result.Succeeded) { - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); return RedirectToAction("Index", new { Message = ManageMessageId.RemovePhoneSuccess }); } } @@ -238,7 +237,7 @@ namespace IdentitySample.Controllers var result = await UserManager.ChangePasswordAsync(user, model.OldPassword, model.NewPassword); if (result.Succeeded) { - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); return RedirectToAction("Index", new { Message = ManageMessageId.ChangePasswordSuccess }); } AddErrors(result); @@ -272,7 +271,7 @@ namespace IdentitySample.Controllers var result = await UserManager.AddPasswordAsync(user, model.NewPassword); if (result.Succeeded) { - await SignInManager.SignInAsync(user, isPersistent: false); + await SignInManager.RefreshSignInAsync(user); return RedirectToAction("Index", new { Message = ManageMessageId.SetPasswordSuccess }); } AddErrors(result); diff --git a/samples/IdentitySample.Mvc/Startup.cs b/samples/IdentitySample.Mvc/Startup.cs index 22ca9c3a45..c30c2f15f7 100644 --- a/samples/IdentitySample.Mvc/Startup.cs +++ b/samples/IdentitySample.Mvc/Startup.cs @@ -7,6 +7,7 @@ using Microsoft.Data.Entity; using Microsoft.Framework.ConfigurationModel; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; +using Microsoft.Framework.Runtime; #if DNX451 using NLog.Config; using NLog.Targets; @@ -16,13 +17,13 @@ namespace IdentitySamples { public partial class Startup { - public Startup() + public Startup(IApplicationEnvironment env) { /* * Below code demonstrates usage of multiple configuration sources. For instance a setting say 'setting1' is found in both the registered sources, * then the later source will win. By this way a Local config can be overridden by a different setting while deployed remotely. */ - Configuration = new Configuration() + Configuration = new Configuration(env.ApplicationBasePath) .AddJsonFile("LocalConfig.json") .AddEnvironmentVariables(); //All environment variables in the process's context flow in as configuration values. } diff --git a/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs b/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs index 12e50cf227..6f36b85d63 100644 --- a/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs +++ b/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs @@ -5,6 +5,7 @@ using System; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNet.Authentication.Cookies; +using Microsoft.AspNet.Http.Authentication; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.OptionsModel; @@ -24,12 +25,7 @@ namespace Microsoft.AspNet.Identity var user = await manager.ValidateSecurityStampAsync(context.Principal, userId); if (user != null) { - var isPersistent = false; - if (context.Properties != null) - { - isPersistent = context.Properties.IsPersistent; - } - await manager.SignInAsync(user, isPersistent, authenticationMethod: null); + await manager.SignInAsync(user, context.Properties, authenticationMethod: null); } else { diff --git a/src/Microsoft.AspNet.Identity/SignInManager.cs b/src/Microsoft.AspNet.Identity/SignInManager.cs index bddec9e11a..6f2756bdea 100644 --- a/src/Microsoft.AspNet.Identity/SignInManager.cs +++ b/src/Microsoft.AspNet.Identity/SignInManager.cs @@ -32,7 +32,6 @@ namespace Microsoft.AspNet.Identity { throw new ArgumentNullException(nameof(userManager)); } - if (contextAccessor == null || contextAccessor.HttpContext == null) { throw new ArgumentNullException(nameof(contextAccessor)); @@ -46,17 +45,15 @@ namespace Microsoft.AspNet.Identity Context = contextAccessor.HttpContext; ClaimsFactory = claimsFactory; Options = optionsAccessor?.Options ?? new IdentityOptions(); - Logger = logger?.CreateLogger>(); } protected internal virtual ILogger Logger { get; set; } - internal UserManager UserManager { get; private set; } - internal HttpContext Context { get; private set; } - internal IUserClaimsPrincipalFactory ClaimsFactory { get; private set; } - internal IdentityOptions Options { get; private set; } + internal UserManager UserManager { get; set; } + internal HttpContext Context { get; set; } + internal IUserClaimsPrincipalFactory ClaimsFactory { get; set; } + internal IdentityOptions Options { get; set; } - // Should this be a func? public virtual async Task CreateUserPrincipalAsync(TUser user) => await ClaimsFactory.CreateAsync(user); @@ -74,7 +71,26 @@ namespace Microsoft.AspNet.Identity return Logger.Log(true); } - public virtual async Task SignInAsync(TUser user, bool isPersistent, string authenticationMethod = null) + /// + /// Called to regenerate the ApplicationCookie for the user, preserving the existing + /// AuthenticationProperties like rememberMe + /// + /// + /// + public virtual async Task RefreshSignInAsync(TUser user) + { + var authResult = await Context.AuthenticateAsync(IdentityOptions.ApplicationCookieAuthenticationScheme); + var properties = authResult?.Properties ?? new AuthenticationProperties(); + var authenticationMethod = authResult?.Principal?.FindFirstValue(ClaimTypes.AuthenticationMethod); + await SignInAsync(user, properties, authenticationMethod); + } + + public virtual Task SignInAsync(TUser user, bool isPersistent, string authenticationMethod = null) + { + return SignInAsync(user, new AuthenticationProperties { IsPersistent = isPersistent }, authenticationMethod); + } + + public virtual async Task SignInAsync(TUser user, AuthenticationProperties authenticationProperties, string authenticationMethod = null) { var userPrincipal = await CreateUserPrincipalAsync(user); // Review: should we guard against CreateUserPrincipal returning null? @@ -84,7 +100,7 @@ namespace Microsoft.AspNet.Identity } Context.Response.SignIn(IdentityOptions.ApplicationCookieAuthenticationScheme, userPrincipal, - new AuthenticationProperties() { IsPersistent = isPersistent }); + authenticationProperties ?? new AuthenticationProperties()); } public virtual void SignOut() @@ -261,13 +277,12 @@ namespace Microsoft.AspNet.Identity { Context.Response.SignOut(IdentityOptions.ExternalCookieAuthenticationScheme); } - await SignInAsync(user, isPersistent, twoFactorInfo.LoginProvider); if (rememberClient) { await RememberTwoFactorClientAsync(user); } await UserManager.ResetAccessFailedCountAsync(user); - await SignInAsync(user, isPersistent); + await SignInAsync(user, isPersistent, twoFactorInfo.LoginProvider); return Logger.Log(SignInResult.Success); } // If the token is incorrect, record the failure which also may cause the user to be locked out @@ -402,7 +417,6 @@ namespace Microsoft.AspNet.Identity return Logger?.BeginScope(state); } - internal static ClaimsPrincipal StoreTwoFactorInfo(string userId, string loginProvider) { var identity = new ClaimsIdentity(IdentityOptions.TwoFactorUserIdCookieAuthenticationType); diff --git a/test/Microsoft.AspNet.Identity.Test/Microsoft.AspNet.Identity.Test.xproj b/test/Microsoft.AspNet.Identity.Test/Microsoft.AspNet.Identity.Test.xproj index e206b5e822..3deb3ffc5b 100644 --- a/test/Microsoft.AspNet.Identity.Test/Microsoft.AspNet.Identity.Test.xproj +++ b/test/Microsoft.AspNet.Identity.Test/Microsoft.AspNet.Identity.Test.xproj @@ -13,5 +13,8 @@ 2.0 + + + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Identity.Test/SecurityStampValidatorTest.cs b/test/Microsoft.AspNet.Identity.Test/SecurityStampValidatorTest.cs index ea06afcf00..6ecd2aecfc 100644 --- a/test/Microsoft.AspNet.Identity.Test/SecurityStampValidatorTest.cs +++ b/test/Microsoft.AspNet.Identity.Test/SecurityStampValidatorTest.cs @@ -44,10 +44,11 @@ namespace Microsoft.AspNet.Identity.Test var httpContext = new Mock(); var contextAccessor = new Mock(); contextAccessor.Setup(a => a.HttpContext).Returns(httpContext.Object); + var properties = new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow, IsPersistent = isPersistent }; var signInManager = new Mock>(userManager.Object, contextAccessor.Object, claimsManager.Object, options.Object, null); signInManager.Setup(s => s.ValidateSecurityStampAsync(It.IsAny(), user.Id)).ReturnsAsync(user).Verifiable(); - signInManager.Setup(s => s.SignInAsync(user, isPersistent, null)).Returns(Task.FromResult(0)).Verifiable(); + signInManager.Setup(s => s.SignInAsync(user, properties, null)).Returns(Task.FromResult(0)).Verifiable(); var services = new ServiceCollection(); services.AddInstance(options.Object); services.AddInstance(signInManager.Object); @@ -57,7 +58,7 @@ namespace Microsoft.AspNet.Identity.Test id.AddClaim(new Claim(ClaimTypes.NameIdentifier, user.Id)); var ticket = new AuthenticationTicket(new ClaimsPrincipal(id), - new AuthenticationProperties { IssuedUtc = DateTimeOffset.UtcNow, IsPersistent = isPersistent }, + properties, IdentityOptions.ApplicationCookieAuthenticationScheme); var context = new CookieValidatePrincipalContext(httpContext.Object, ticket, new CookieAuthenticationOptions()); Assert.NotNull(context.Properties); diff --git a/test/Microsoft.AspNet.Identity.Test/SignInManagerTest.cs b/test/Microsoft.AspNet.Identity.Test/SignInManagerTest.cs index 7609c38f4c..30dcb3c55b 100644 --- a/test/Microsoft.AspNet.Identity.Test/SignInManagerTest.cs +++ b/test/Microsoft.AspNet.Identity.Test/SignInManagerTest.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; using System.Security.Claims; -using System.Security.Principal; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Hosting; @@ -344,6 +343,42 @@ namespace Microsoft.AspNet.Identity.Test response.Verify(); } + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public async Task CanResignIn(bool isPersistent, bool externalLogin) + { + // Setup + var user = new TestUser { UserName = "Foo" }; + var context = new Mock(); + var loginProvider = "loginprovider"; + var id = new ClaimsIdentity(); + if (externalLogin) + { + id.AddClaim(new Claim(ClaimTypes.AuthenticationMethod, loginProvider)); + } + var properties = new AuthenticationProperties { IsPersistent = isPersistent }; + var authResult = new AuthenticationResult(new ClaimsPrincipal(id), properties, new AuthenticationDescription()); + context.Setup(c => c.AuthenticateAsync(IdentityOptions.ApplicationCookieAuthenticationScheme)).ReturnsAsync(authResult).Verifiable(); + var manager = SetupUserManager(user); + var signInManager = new Mock>(manager.Object, + new HttpContextAccessor { HttpContext = context.Object }, + new Mock>().Object, + null, null) + { CallBase = true }; + signInManager.Setup(s => s.SignInAsync(user, properties, externalLogin ? loginProvider : null)).Returns(Task.FromResult(0)).Verifiable(); + signInManager.Object.Context = context.Object; + + // Act + await signInManager.Object.RefreshSignInAsync(user); + + // Assert + context.Verify(); + signInManager.Verify(); + } + [Theory] [InlineData(true, true, true, true)] [InlineData(true, true, false, true)]