Clone tickets for sliding refresh #1607

src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs

@@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
         private DateTimeOffset? _refreshExpiresUtc;
         private string _sessionKey;
         private Task<AuthenticateResult> _readCookieTask;
+        private AuthenticationTicket _refreshTicket;
 
         public CookieAuthenticationHandler(IOptionsMonitor<CookieAuthenticationOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
             : base(options, logger, encoder, clock)
@@ -99,9 +100,27 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
                 _refreshIssuedUtc = currentUtc;
                 var timeSpan = expiresUtc.Value.Subtract(issuedUtc.Value);
                 _refreshExpiresUtc = currentUtc.Add(timeSpan);
+                _refreshTicket = CloneTicket(ticket);
             }
         }
 
+        private AuthenticationTicket CloneTicket(AuthenticationTicket ticket)
+        {
+            var newPrincipal = new ClaimsPrincipal();
+            foreach (var identity in ticket.Principal.Identities)
+            {
+                newPrincipal.AddIdentity(identity.Clone());
+            }
+
+            var newProperties = new AuthenticationProperties();
+            foreach (var item in ticket.Properties.Items)
+            {
+                newProperties.Items[item.Key] = item.Value;
+            }
+
+            return new AuthenticationTicket(newPrincipal, newProperties, ticket.AuthenticationScheme);
+        }
+
         private async Task<AuthenticateResult> ReadCookieTicket()
         {
             var cookie = Options.CookieManager.GetRequestCookie(Context, Options.Cookie.Name);
@@ -190,7 +209,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
                 return;
             }
 
-            var ticket = (await HandleAuthenticateOnceSafeAsync())?.Ticket;
+            var ticket = _refreshTicket;
             if (ticket != null)
             {
                 var properties = ticket.Properties;

test/Microsoft.AspNetCore.Authentication.Test/CookieTests.cs

@@ -515,8 +515,10 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
 
         private Task SignInAsAlice(HttpContext context)
         {
+            var user = new ClaimsIdentity(new GenericIdentity("Alice", "Cookies"));
+            user.AddClaim(new Claim("marker", "true"));
             return context.SignInAsync("Cookies",
-                new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity("Alice", "Cookies"))),
+                new ClaimsPrincipal(user),
                 new AuthenticationProperties());
         }
 
@@ -942,6 +944,61 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
             Assert.Null(FindClaimValue(transaction5, ClaimTypes.Name));
         }
 
+        [Fact]
+        public async Task CookieCanBeRenewedByValidatorWithModifiedProperties()
+        {
+            var server = CreateServer(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"));
+                        }
+                        return Task.FromResult(0);
+                    }
+                };
+            },
+            context =>
+                context.SignInAsync("Cookies",
+                    new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity("Alice", "Cookies")))));
+
+            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(5));
+
+            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(6));
+
+            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(11));
+
+            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()
         {
@@ -1114,6 +1171,51 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
             Assert.Equal("Alice", FindClaimValue(transaction5, ClaimTypes.Name));
         }
 
+        [Fact]
+        public async Task CookieIsRenewedWithSlidingExpirationWithoutTransformations()
+        {
+            var server = CreateServer(o =>
+            {
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
+                o.SlidingExpiration = true;
+                o.Events.OnValidatePrincipal = c =>
+                {
+                    // https://github.com/aspnet/Security/issues/1607
+                    // On sliding refresh the transformed principal should not be serialized into the cookie, only the original principal.
+                    Assert.Single(c.Principal.Identities);
+                    Assert.True(c.Principal.Identities.First().HasClaim("marker", "true"));
+                    return Task.CompletedTask;
+                };
+            },
+            SignInAsAlice,
+            claimsTransform: true);
+
+            var transaction1 = await SendAsync(server, "http://example.com/testpath");
+
+            var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            Assert.Null(transaction2.SetCookie);
+            Assert.Equal("Alice", FindClaimValue(transaction2, ClaimTypes.Name));
+
+            _clock.Add(TimeSpan.FromMinutes(4));
+
+            var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            Assert.Null(transaction3.SetCookie);
+            Assert.Equal("Alice", FindClaimValue(transaction3, ClaimTypes.Name));
+
+            _clock.Add(TimeSpan.FromMinutes(4));
+
+            // transaction4 should arrive with a new SetCookie value
+            var transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            Assert.NotNull(transaction4.SetCookie);
+            Assert.Equal("Alice", FindClaimValue(transaction4, ClaimTypes.Name));
+
+            _clock.Add(TimeSpan.FromMinutes(4));
+
+            var transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue);
+            Assert.Null(transaction5.SetCookie);
+            Assert.Equal("Alice", FindClaimValue(transaction5, ClaimTypes.Name));
+        }
+
         [Fact]
         public async Task CookieUsesPathBaseByDefault()
         {
@@ -1643,6 +1745,13 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
         {
             public Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal p)
             {
+                var firstId = p.Identities.First();
+                if (firstId.HasClaim("marker", "true"))
+                {
+                    firstId.RemoveClaim(firstId.FindFirst("marker"));
+                }
+                // TransformAsync could be called twice on one request if you have a default scheme and also
+                // call AuthenticateAsync.
                 if (!p.Identities.Any(i => i.AuthenticationType == "xform"))
                 {
                     var id = new ClaimsIdentity("xform");
@@ -1658,7 +1767,10 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
             {
                 s.AddSingleton<ISystemClock>(_clock);
                 s.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie(configureOptions);
-                s.AddSingleton<IClaimsTransformation, ClaimsTransformer>();
+                if (claimsTransform)
+                {
+                    s.AddSingleton<IClaimsTransformation, ClaimsTransformer>();
+                }
             }, testpath, baseAddress);
 
         private static TestServer CreateServerWithServices(Action<IServiceCollection> configureServices, Func<HttpContext, Task> testpath = null, Uri baseAddress = null)