diff --git a/src/Identity/Core/src/SignInManager.cs b/src/Identity/Core/src/SignInManager.cs index b5659b329854..66f06c4d3465 100644 --- a/src/Identity/Core/src/SignInManager.cs +++ b/src/Identity/Core/src/SignInManager.cs @@ -162,8 +162,21 @@ public virtual async Task CanSignInAsync(TUser user) public virtual async Task RefreshSignInAsync(TUser user) { var auth = await Context.AuthenticateAsync(AuthenticationScheme); - IList claims = Array.Empty(); + if (!auth.Succeeded || auth.Principal?.Identity?.IsAuthenticated != true) + { + Logger.LogError("RefreshSignInAsync prevented because the user is not currently authenticated. Use SignInAsync instead for initial sign in."); + return; + } + var authenticatedUserId = UserManager.GetUserId(auth.Principal); + var newUserId = await UserManager.GetUserIdAsync(user); + if (authenticatedUserId == null || authenticatedUserId != newUserId) + { + Logger.LogError("RefreshSignInAsync prevented because currently authenticated user has a different UserId. Use SignInAsync instead to change users."); + return; + } + + IList claims = Array.Empty(); var authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); var amr = auth?.Principal?.FindFirst("amr"); diff --git a/src/Identity/test/Identity.Test/SignInManagerTest.cs b/src/Identity/test/Identity.Test/SignInManagerTest.cs index d1072676138a..73fe6d6be218 100644 --- a/src/Identity/test/Identity.Test/SignInManagerTest.cs +++ b/src/Identity/test/Identity.Test/SignInManagerTest.cs @@ -592,38 +592,38 @@ public async Task CanExternalSignIn(bool isPersistent, bool supportsLockout) [InlineData(true, false)] [InlineData(false, true)] [InlineData(false, false)] - public async Task CanResignIn( - // Suppress warning that says theory methods should use all of their parameters. - // See comments below about why this isn't used. -#pragma warning disable xUnit1026 - bool isPersistent, -#pragma warning restore xUnit1026 - bool externalLogin) + public async Task CanResignIn(bool isPersistent, bool externalLogin) { // Setup var user = new PocoUser { UserName = "Foo" }; var context = new DefaultHttpContext(); var auth = MockAuth(context); var loginProvider = "loginprovider"; - var id = new ClaimsIdentity(); + var id = new ClaimsIdentity("authscheme"); if (externalLogin) { id.AddClaim(new Claim(ClaimTypes.AuthenticationMethod, loginProvider)); } - // REVIEW: auth changes we lost the ability to mock is persistent - //var properties = new AuthenticationProperties { IsPersistent = isPersistent }; - var authResult = AuthenticateResult.NoResult(); + + var claimsPrincipal = new ClaimsPrincipal(id); + var properties = new AuthenticationProperties { IsPersistent = isPersistent }; + var authResult = AuthenticateResult.Success(new AuthenticationTicket(claimsPrincipal, properties, "authscheme")); auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.ApplicationScheme)) .Returns(Task.FromResult(authResult)).Verifiable(); var manager = SetupUserManager(user); + manager.Setup(m => m.GetUserId(claimsPrincipal)).Returns(user.Id.ToString()); var signInManager = new Mock>(manager.Object, new HttpContextAccessor { HttpContext = context }, new Mock>().Object, null, null, new Mock().Object, null) { 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.SignInWithClaimsAsync(user, It.IsAny(), It.IsAny>())).Returns(Task.FromResult(0)).Verifiable(); + + signInManager.Setup(s => s.SignInWithClaimsAsync(user, + It.Is(properties => properties.IsPersistent == isPersistent), + It.Is>(claims => !externalLogin || + claims.Any(claim => claim.Type == ClaimTypes.AuthenticationMethod && claim.Value == loginProvider)))) + .Returns(Task.FromResult(0)).Verifiable(); + signInManager.Object.Context = context; // Act @@ -634,6 +634,58 @@ public async Task CanResignIn( signInManager.Verify(); } + [Fact] + public async Task ResignInNoOpsAndLogsErrorIfNotAuthenticated() + { + var user = new PocoUser { UserName = "Foo" }; + var context = new DefaultHttpContext(); + var auth = MockAuth(context); + var manager = SetupUserManager(user); + var logger = new TestLogger>(); + var signInManager = new Mock>(manager.Object, + new HttpContextAccessor { HttpContext = context }, + new Mock>().Object, + null, logger, new Mock().Object, null) + { CallBase = true }; + auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.ApplicationScheme)) + .Returns(Task.FromResult(AuthenticateResult.NoResult())).Verifiable(); + + await signInManager.Object.RefreshSignInAsync(user); + + Assert.Contains("RefreshSignInAsync prevented because the user is not currently authenticated. Use SignInAsync instead for initial sign in.", logger.LogMessages); + auth.Verify(); + signInManager.Verify(s => s.SignInWithClaimsAsync(It.IsAny(), It.IsAny(), It.IsAny>()), + Times.Never()); + } + + [Fact] + public async Task ResignInNoOpsAndLogsErrorIfAuthenticatedWithDifferentUser() + { + var user = new PocoUser { UserName = "Foo" }; + var context = new DefaultHttpContext(); + var auth = MockAuth(context); + var manager = SetupUserManager(user); + var logger = new TestLogger>(); + var signInManager = new Mock>(manager.Object, + new HttpContextAccessor { HttpContext = context }, + new Mock>().Object, + null, logger, new Mock().Object, null) + { CallBase = true }; + var id = new ClaimsIdentity("authscheme"); + var claimsPrincipal = new ClaimsPrincipal(id); + var authResult = AuthenticateResult.Success(new AuthenticationTicket(claimsPrincipal, new AuthenticationProperties(), "authscheme")); + auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.ApplicationScheme)) + .Returns(Task.FromResult(authResult)).Verifiable(); + manager.Setup(m => m.GetUserId(claimsPrincipal)).Returns("different"); + + await signInManager.Object.RefreshSignInAsync(user); + + Assert.Contains("RefreshSignInAsync prevented because currently authenticated user has a different UserId. Use SignInAsync instead to change users.", logger.LogMessages); + auth.Verify(); + signInManager.Verify(s => s.SignInWithClaimsAsync(It.IsAny(), It.IsAny(), It.IsAny>()), + Times.Never()); + } + [Theory] [InlineData(true, true, true, true)] [InlineData(true, true, false, true)]