Merge pull request #60878 from dotnet/wtgodbe/MergeInt23

[release/2.3] Merge from internal

Author: wtgodbe

eng/PatchConfig.props

@@ -15,6 +15,7 @@ Later on, this will be checked using this condition:
 
   <PropertyGroup Condition=" '$(VersionPrefix)' == '2.3.1' ">
     <PackagesInPatch>
+      Microsoft.AspNetCore.Identity;
     </PackagesInPatch>
   </PropertyGroup>
 </Project>

src/Identity/Core/src/SignInManager.cs

@@ -162,6 +162,19 @@ public virtual async Task<bool> CanSignInAsync(TUser user)
         public virtual async Task RefreshSignInAsync(TUser user)
         {
             var auth = await Context.AuthenticateAsync(IdentityConstants.ApplicationScheme);
+            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;
+            }
+
             var authenticationMethod = auth?.Principal?.FindFirstValue(ClaimTypes.AuthenticationMethod);
             await SignInAsync(user, auth?.Properties, authenticationMethod);
         }

src/Identity/test/Identity.Test/SignInManagerTest.cs

@@ -548,38 +548,37 @@ 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<SignInManager<PocoUser>>(manager.Object,
                 new HttpContextAccessor { HttpContext = context },
                 new Mock<IUserClaimsPrincipalFactory<PocoUser>>().Object,
                 null, null, new Mock<IAuthenticationSchemeProvider>().Object)
             { CallBase = true };
-            //signInManager.Setup(s => s.SignInAsync(user, It.Is<AuthenticationProperties>(p => p.IsPersistent == isPersistent),
-            //externalLogin? loginProvider : null)).Returns(Task.FromResult(0)).Verifiable();
-            signInManager.Setup(s => s.SignInAsync(user, It.IsAny<AuthenticationProperties>(), null)).Returns(Task.FromResult(0)).Verifiable();
+
+            signInManager.Setup(s => s.SignInAsync(user,
+                    It.Is<AuthenticationProperties>(p => p.IsPersistent == isPersistent),
+                    externalLogin ? loginProvider : null))
+                .Returns(Task.FromResult(0)).Verifiable();
+
             signInManager.Object.Context = context;
 
             // Act
@@ -590,6 +589,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<SignInManager<PocoUser>>();
+            var signInManager = new Mock<SignInManager<PocoUser>>(manager.Object,
+                new HttpContextAccessor { HttpContext = context },
+                new Mock<IUserClaimsPrincipalFactory<PocoUser>>().Object,
+                null, logger, new Mock<IAuthenticationSchemeProvider>().Object)
+            { 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.SignInAsync(It.IsAny<PocoUser>(), It.IsAny<AuthenticationProperties>(), It.IsAny<string>()),
+                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<SignInManager<PocoUser>>();
+            var signInManager = new Mock<SignInManager<PocoUser>>(manager.Object,
+                new HttpContextAccessor { HttpContext = context },
+                new Mock<IUserClaimsPrincipalFactory<PocoUser>>().Object,
+                null, logger, new Mock<IAuthenticationSchemeProvider>().Object)
+            { 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.SignInAsync(It.IsAny<PocoUser>(), It.IsAny<AuthenticationProperties>(), It.IsAny<string>()),
+                Times.Never());
+        }
+
         [Theory]
         [InlineData(true, true, true, true)]
         [InlineData(true, true, false, true)]