From c769dd7736e2a12ca187da3295056243beb86cb7 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 31 May 2019 22:52:47 -0700 Subject: [PATCH] Add SignInWithClaims overload and set amr claim appropriately (#10636) --- ...osoft.AspNetCore.Identity.netcoreapp3.0.cs | 4 +- src/Identity/Core/src/SignInManager.cs | 68 ++++++++++++++++--- .../test/Identity.Test/SignInManagerTest.cs | 19 +++--- 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/src/Identity/Core/ref/Microsoft.AspNetCore.Identity.netcoreapp3.0.cs b/src/Identity/Core/ref/Microsoft.AspNetCore.Identity.netcoreapp3.0.cs index 2efe1f5634..a08fa9b444 100644 --- a/src/Identity/Core/ref/Microsoft.AspNetCore.Identity.netcoreapp3.0.cs +++ b/src/Identity/Core/ref/Microsoft.AspNetCore.Identity.netcoreapp3.0.cs @@ -148,12 +148,14 @@ namespace Microsoft.AspNetCore.Identity [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task RememberTwoFactorClientAsync(TUser user) { throw null; } protected virtual System.Threading.Tasks.Task ResetLockout(TUser user) { throw null; } - [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task SignInAsync(TUser user, Microsoft.AspNetCore.Authentication.AuthenticationProperties authenticationProperties, string authenticationMethod = null) { throw null; } public virtual System.Threading.Tasks.Task SignInAsync(TUser user, bool isPersistent, string authenticationMethod = null) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] protected virtual System.Threading.Tasks.Task SignInOrTwoFactorAsync(TUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] + public virtual System.Threading.Tasks.Task SignInWithClaimsAsync(TUser user, Microsoft.AspNetCore.Authentication.AuthenticationProperties authenticationProperties, System.Collections.Generic.IEnumerable additionalClaims) { throw null; } + public virtual System.Threading.Tasks.Task SignInWithClaimsAsync(TUser user, bool isPersistent, System.Collections.Generic.IEnumerable additionalClaims) { throw null; } + [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task SignOutAsync() { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] public virtual System.Threading.Tasks.Task TwoFactorAuthenticatorSignInAsync(string code, bool isPersistent, bool rememberClient) { throw null; } diff --git a/src/Identity/Core/src/SignInManager.cs b/src/Identity/Core/src/SignInManager.cs index 1fa03c3d8a..3435099abc 100644 --- a/src/Identity/Core/src/SignInManager.cs +++ b/src/Identity/Core/src/SignInManager.cs @@ -169,8 +169,19 @@ namespace Microsoft.AspNetCore.Identity public virtual async Task RefreshSignInAsync(TUser user) { var auth = await Context.AuthenticateAsync(IdentityConstants.ApplicationScheme); - var authenticationMethod = auth?.Principal?.FindFirstValue(ClaimTypes.AuthenticationMethod); - await SignInAsync(user, auth?.Properties, authenticationMethod); + var claims = new List(); + var authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); + if (authenticationMethod != null) + { + claims.Add(authenticationMethod); + } + var amr = auth?.Principal?.FindFirst("amr"); + if (amr != null) + { + claims.Add(amr); + } + + await SignInWithClaimsAsync(user, auth?.Properties, claims); } /// @@ -181,9 +192,7 @@ namespace Microsoft.AspNetCore.Identity /// Name of the method used to authenticate the user. /// The task object representing the asynchronous operation. public virtual Task SignInAsync(TUser user, bool isPersistent, string authenticationMethod = null) - { - return SignInAsync(user, new AuthenticationProperties { IsPersistent = isPersistent }, authenticationMethod); - } + => SignInAsync(user, new AuthenticationProperties { IsPersistent = isPersistent }, authenticationMethod); /// /// Signs in the specified . @@ -192,13 +201,39 @@ namespace Microsoft.AspNetCore.Identity /// Properties applied to the login and authentication cookie. /// Name of the method used to authenticate the user. /// The task object representing the asynchronous operation. - public virtual async Task SignInAsync(TUser user, AuthenticationProperties authenticationProperties, string authenticationMethod = null) + public virtual Task SignInAsync(TUser user, AuthenticationProperties authenticationProperties, string authenticationMethod = null) { - var userPrincipal = await CreateUserPrincipalAsync(user); - // Review: should we guard against CreateUserPrincipal returning null? + var additionalClaims = new List(); if (authenticationMethod != null) { - userPrincipal.Identities.First().AddClaim(new Claim(ClaimTypes.AuthenticationMethod, authenticationMethod)); + additionalClaims.Add(new Claim(ClaimTypes.AuthenticationMethod, authenticationMethod)); + } + return SignInWithClaimsAsync(user, authenticationProperties, additionalClaims); + } + + /// + /// Signs in the specified . + /// + /// The user to sign-in. + /// Flag indicating whether the sign-in cookie should persist after the browser is closed. + /// Additional claims that will be stored in the cookie. + /// The task object representing the asynchronous operation. + public virtual Task SignInWithClaimsAsync(TUser user, bool isPersistent, IEnumerable additionalClaims) + => SignInWithClaimsAsync(user, new AuthenticationProperties { IsPersistent = isPersistent }, additionalClaims); + + /// + /// Signs in the specified . + /// + /// The user to sign-in. + /// Properties applied to the login and authentication cookie. + /// Additional claims that will be stored in the cookie. + /// The task object representing the asynchronous operation. + public virtual async Task SignInWithClaimsAsync(TUser user, AuthenticationProperties authenticationProperties, IEnumerable additionalClaims) + { + var userPrincipal = await CreateUserPrincipalAsync(user); + foreach (var claim in additionalClaims) + { + userPrincipal.Identities.First().AddClaim(claim); } await Context.SignInAsync(IdentityConstants.ApplicationScheme, userPrincipal, @@ -438,9 +473,13 @@ namespace Microsoft.AspNetCore.Identity // When token is verified correctly, clear the access failed count used for lockout await ResetLockout(user); + var claims = new List(); + claims.Add(new Claim("amr", "mfa")); + // Cleanup external cookie if (twoFactorInfo.LoginProvider != null) { + claims.Add(new Claim(ClaimTypes.AuthenticationMethod, twoFactorInfo.LoginProvider)); await Context.SignOutAsync(IdentityConstants.ExternalScheme); } // Cleanup two factor user id cookie @@ -449,7 +488,7 @@ namespace Microsoft.AspNetCore.Identity { await RememberTwoFactorClientAsync(user); } - await SignInAsync(user, isPersistent, twoFactorInfo.LoginProvider); + await SignInWithClaimsAsync(user, isPersistent, claims); } /// @@ -760,7 +799,14 @@ namespace Microsoft.AspNetCore.Identity { await Context.SignOutAsync(IdentityConstants.ExternalScheme); } - await SignInAsync(user, isPersistent, loginProvider); + if (loginProvider == null) + { + await SignInWithClaimsAsync(user, isPersistent, new Claim[] { new Claim("amr", "pwd") }); + } + else + { + await SignInAsync(user, isPersistent, loginProvider); + } return SignInResult.Success; } diff --git a/src/Identity/test/Identity.Test/SignInManagerTest.cs b/src/Identity/test/Identity.Test/SignInManagerTest.cs index 636df0e3f8..007942d424 100644 --- a/src/Identity/test/Identity.Test/SignInManagerTest.cs +++ b/src/Identity/test/Identity.Test/SignInManagerTest.cs @@ -202,10 +202,9 @@ namespace Microsoft.AspNetCore.Identity.Test manager.Setup(m => m.SupportsUserLockout).Returns(true).Verifiable(); manager.Setup(m => m.IsLockedOutAsync(user)).ReturnsAsync(false).Verifiable(); manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable(); - var context = new DefaultHttpContext(); var auth = MockAuth(context); - SetupSignIn(context, auth, user.Id, isPersistent); + SetupSignIn(context, auth, user.Id, isPersistent, loginProvider: null, amr: "pwd"); var helper = SetupSignInManager(manager.Object, context); // Act @@ -229,7 +228,7 @@ namespace Microsoft.AspNetCore.Identity.Test var context = new DefaultHttpContext(); var auth = MockAuth(context); - SetupSignIn(context, auth, user.Id, false); + SetupSignIn(context, auth, user.Id, false, loginProvider: null, amr: "pwd"); var helper = SetupSignInManager(manager.Object, context); // Act @@ -579,7 +578,7 @@ namespace Microsoft.AspNetCore.Identity.Test { CallBase = true }; //signInManager.Setup(s => s.SignInAsync(user, It.Is(p => p.IsPersistent == isPersistent), //externalLogin? loginProvider : null)).Returns(Task.FromResult(0)).Verifiable(); - signInManager.Setup(s => s.SignInAsync(user, It.IsAny(), null)).Returns(Task.FromResult(0)).Verifiable(); + signInManager.Setup(s => s.SignInWithClaimsAsync(user, It.IsAny(), It.IsAny>())).Returns(Task.FromResult(0)).Verifiable(); signInManager.Object.Context = context; // Act @@ -632,6 +631,7 @@ namespace Microsoft.AspNetCore.Identity.Test auth.Setup(a => a.SignInAsync(context, IdentityConstants.ApplicationScheme, It.Is(i => i.FindFirstValue(ClaimTypes.AuthenticationMethod) == loginProvider + && i.FindFirstValue("amr") == "mfa" && i.FindFirstValue(ClaimTypes.NameIdentifier) == user.Id), It.IsAny())).Returns(Task.FromResult(0)).Verifiable(); // REVIEW: restore ability to test is persistent @@ -641,7 +641,7 @@ namespace Microsoft.AspNetCore.Identity.Test } else { - SetupSignIn(context, auth, user.Id); + SetupSignIn(context, auth, user.Id, isPersistent, null, "mfa"); } if (rememberClient) { @@ -864,7 +864,7 @@ namespace Microsoft.AspNetCore.Identity.Test if (confirmed) { manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable(); - SetupSignIn(context, auth); + SetupSignIn(context, auth, user.Id, isPersistent: null, loginProvider: null, amr: "pwd"); } var identityOptions = new IdentityOptions(); identityOptions.SignIn.RequireConfirmedEmail = true; @@ -893,13 +893,14 @@ namespace Microsoft.AspNetCore.Identity.Test auth.Verify(); } - private static void SetupSignIn(HttpContext context, Mock auth, string userId = null, bool? isPersistent = null, string loginProvider = null) + private static void SetupSignIn(HttpContext context, Mock auth, string userId = null, bool? isPersistent = null, string loginProvider = null, string amr = null) { auth.Setup(a => a.SignInAsync(context, IdentityConstants.ApplicationScheme, It.Is(id => (userId == null || id.FindFirstValue(ClaimTypes.NameIdentifier) == userId) && - (loginProvider == null || id.FindFirstValue(ClaimTypes.AuthenticationMethod) == loginProvider)), + (loginProvider == null || id.FindFirstValue(ClaimTypes.AuthenticationMethod) == loginProvider) && + (amr == null || id.FindFirstValue("amr") == amr)), It.Is(v => isPersistent == null || v.IsPersistent == isPersistent))).Returns(Task.FromResult(0)).Verifiable(); } @@ -917,7 +918,7 @@ namespace Microsoft.AspNetCore.Identity.Test if (confirmed) { manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable(); - SetupSignIn(context, auth); + SetupSignIn(context, auth, user.Id, isPersistent: null, loginProvider: null, amr: "pwd"); } var identityOptions = new IdentityOptions();