[release/5.0] Honor sliding expiration in the security stamp validator (#36468)

* Honor sliding expiration in the security stamp validator

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Aditya Mandaleeka <[email protected]>

* Remove redundant null check

Co-authored-by: Chris R <[email protected]>
Co-authored-by: Aditya Mandaleeka <[email protected]>

Author: 3 people

src/Identity/Core/src/SecurityStampValidator.cs

@@ -90,6 +90,13 @@ protected virtual async Task SecurityStampVerified(TUser user, CookieValidatePri
             // REVIEW: note we lost login authentication method
             context.ReplacePrincipal(newPrincipal);
             context.ShouldRenew = true;
+
+            if (!context.Options.SlidingExpiration)
+            {
+                // On renewal calculate the new ticket length relative to now to avoid
+                // extending the expiration.
+                context.Properties.IssuedUtc = Clock.UtcNow;
+            }
         }
 
         /// <summary>
@@ -110,7 +117,7 @@ protected virtual Task<TUser> VerifySecurityStamp(ClaimsPrincipal principal)
         public virtual async Task ValidateAsync(CookieValidatePrincipalContext context)
         {
             var currentUtc = DateTimeOffset.UtcNow;
-            if (context.Options != null && Clock != null)
+            if (Clock != null)
             {
                 currentUtc = Clock.UtcNow;
             }

src/Identity/test/Identity.Test/SecurityStampValidatorTest.cs

@@ -264,6 +264,53 @@ public async Task OnValidateIdentityDoesNotRejectsWhenNotExpired()
             Assert.NotNull(context.Principal);
         }
 
+        [Fact]
+        public async Task OnValidateIdentityDoesNotExtendExpirationWhenSlidingIsDisabled()
+        {
+            var user = new PocoUser("test");
+            var httpContext = new Mock<HttpContext>();
+            var userManager = MockHelpers.MockUserManager<PocoUser>();
+            var identityOptions = new Mock<IOptions<IdentityOptions>>();
+            identityOptions.Setup(a => a.Value).Returns(new IdentityOptions());
+            var claimsManager = new Mock<IUserClaimsPrincipalFactory<PocoUser>>();
+            var options = new Mock<IOptions<SecurityStampValidatorOptions>>();
+            options.Setup(a => a.Value).Returns(new SecurityStampValidatorOptions { ValidationInterval = TimeSpan.FromMinutes(1) });
+            var contextAccessor = new Mock<IHttpContextAccessor>();
+            contextAccessor.Setup(a => a.HttpContext).Returns(httpContext.Object);
+            var signInManager = new Mock<SignInManager<PocoUser>>(userManager.Object,
+                contextAccessor.Object, claimsManager.Object, identityOptions.Object, null, new Mock<IAuthenticationSchemeProvider>().Object, new DefaultUserConfirmation<PocoUser>());
+            signInManager.Setup(s => s.ValidateSecurityStampAsync(It.IsAny<ClaimsPrincipal>())).Returns(Task.FromResult(user));
+            signInManager.Setup(s => s.CreateUserPrincipalAsync(It.IsAny<PocoUser>())).Returns(Task.FromResult(new ClaimsPrincipal(new ClaimsIdentity("auth"))));
+            signInManager.Setup(s => s.SignInAsync(user, false, null)).Throws(new Exception("Shouldn't be called"));
+            var services = new ServiceCollection();
+            services.AddSingleton(options.Object);
+            services.AddSingleton(signInManager.Object);
+            var clock = new SystemClock();
+            services.AddSingleton<ISecurityStampValidator>(new SecurityStampValidator<PocoUser>(options.Object, signInManager.Object, clock, new LoggerFactory()));
+            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 = clock.UtcNow - TimeSpan.FromDays(1),
+                    ExpiresUtc = clock.UtcNow + TimeSpan.FromDays(1),
+                },
+                IdentityConstants.ApplicationScheme);
+            var context = new CookieValidatePrincipalContext(httpContext.Object, new AuthenticationSchemeBuilder(IdentityConstants.ApplicationScheme) { HandlerType = typeof(NoopHandler) }.Build(),
+                new CookieAuthenticationOptions() { SlidingExpiration = false }, ticket);
+            Assert.NotNull(context.Properties);
+            Assert.NotNull(context.Options);
+            Assert.NotNull(context.Principal);
+            await SecurityStampValidator.ValidatePrincipalAsync(context);
+
+            // Issued is moved forward, expires is not.
+            Assert.Equal(clock.UtcNow, context.Properties.IssuedUtc);
+            Assert.Equal(clock.UtcNow + TimeSpan.FromDays(1), context.Properties.ExpiresUtc);
+            Assert.NotNull(context.Principal);
+        }
+
         private async Task RunRememberClientCookieTest(bool shouldStampValidate, bool validationSuccess)
         {
             var user = new PocoUser("test");

src/Security/Authentication/test/CookieTests.cs

@@ -713,6 +713,65 @@ public async Task CookieCanBeRenewedByValidatorWithModifiedProperties()
             Assert.Null(FindClaimValue(transaction5, "counter"));
         }
 
+        [Fact]
+        public async Task CookieCanBeRenewedByValidatorWithModifiedLifetime()
+        {
+            using var host = await CreateHost(o =>
+            {
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
+                o.Events = new CookieAuthenticationEvents
+                {
+                    OnValidatePrincipal = ctx =>
+                    {
+                        ctx.ShouldRenew = true;
+                        var id = ctx.Principal.Identities.First();
+                        var claim = id.FindFirst("counter");
+                        if (claim == null)
+                        {
+                            id.AddClaim(new Claim("counter", "1"));
+                        }
+                        else
+                        {
+                            id.RemoveClaim(claim);
+                            id.AddClaim(new Claim("counter", claim.Value + "1"));
+                        }
+                        // Causes the expiry time to not be extended because the lifetime is
+                        // calculated relative to the issue time.
+                        ctx.Properties.IssuedUtc = _clock.UtcNow;
+                        return Task.FromResult(0);
+                    }
+                };
+            },
+            context =>
+                context.SignInAsync("Cookies",
+                    new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity("Alice", "Cookies")))));
+
+            using var server = host.GetTestServer();
+            var transaction1 = await SendAsync(server, "http://example.com/testpath");
+
+            var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            Assert.NotNull(transaction2.SetCookie);
+            Assert.Equal("1", FindClaimValue(transaction2, "counter"));
+
+            _clock.Add(TimeSpan.FromMinutes(1));
+
+            var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction2.CookieNameValue);
+            Assert.NotNull(transaction3.SetCookie);
+            Assert.Equal("11", FindClaimValue(transaction3, "counter"));
+
+            _clock.Add(TimeSpan.FromMinutes(1));
+
+            var transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction3.CookieNameValue);
+            Assert.NotNull(transaction4.SetCookie);
+            Assert.Equal("111", FindClaimValue(transaction4, "counter"));
+
+            _clock.Add(TimeSpan.FromMinutes(9));
+
+            var transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue);
+            Assert.Null(transaction5.SetCookie);
+            Assert.Null(FindClaimValue(transaction5, "counter"));
+        }
+
         [Fact]
         public async Task CookieValidatorOnlyCalledOnce()
         {