Cookie fixes

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

@@ -24,17 +24,17 @@ namespace Microsoft.AspNet.Authentication.Cookies
         private const string SessionIdClaim = "Microsoft.AspNet.Authentication.Cookies-SessionId";
 
         private bool _shouldRenew;
-        private DateTimeOffset _renewIssuedUtc;
+        private DateTimeOffset? _renewIssuedUtc;
-        private DateTimeOffset _renewExpiresUtc;
+        private DateTimeOffset? _renewExpiresUtc;
         private string _sessionKey;
 
-        public override async Task<AuthenticationTicket> AuthenticateAsync()
+        protected override async Task<AuthenticationTicket> AuthenticateAsync()
         {
             AuthenticationTicket ticket = null;
             try
             {
                 var cookie = Options.CookieManager.GetRequestCookie(Context, Options.CookieName);
-                if (string.IsNullOrWhiteSpace(cookie))
+                if (string.IsNullOrEmpty(cookie))
                 {
                     return null;
                 }
@@ -96,6 +96,11 @@ namespace Microsoft.AspNet.Authentication.Cookies
 
                 await Options.Notifications.ValidatePrincipal(context);
 
+                if (context.ShouldRenew)
+                {
+                    _shouldRenew = true;
+                }
+
                 return new AuthenticationTicket(context.Principal, context.Properties, Options.AuthenticationScheme);
             }
             catch (Exception exception)
@@ -130,28 +135,33 @@ namespace Microsoft.AspNet.Authentication.Cookies
             return cookieOptions;
         }
 
-        private async Task ApplyCookie(AuthenticationTicket model)
+        private async Task ApplyCookie(AuthenticationTicket ticket)
         {
-            var cookieOptions = BuildCookieOptions();
+            if (_renewIssuedUtc.HasValue)
-
+            {
-            model.Properties.IssuedUtc = _renewIssuedUtc;
+                ticket.Properties.IssuedUtc = _renewIssuedUtc;
-            model.Properties.ExpiresUtc = _renewExpiresUtc;
+            }
+            if (_renewExpiresUtc.HasValue)
+            {
+                ticket.Properties.ExpiresUtc = _renewExpiresUtc;
+            }
 
             if (Options.SessionStore != null && _sessionKey != null)
             {
-                await Options.SessionStore.RenewAsync(_sessionKey, model);
+                await Options.SessionStore.RenewAsync(_sessionKey, ticket);
                 var principal = new ClaimsPrincipal(
                     new ClaimsIdentity(
                         new[] { new Claim(SessionIdClaim, _sessionKey, ClaimValueTypes.String, Options.ClaimsIssuer) },
                         Options.AuthenticationScheme));
-                model = new AuthenticationTicket(principal, null, Options.AuthenticationScheme);
+                ticket = new AuthenticationTicket(principal, null, Options.AuthenticationScheme);
             }
 
-            var cookieValue = Options.TicketDataFormat.Protect(model);
+            var cookieValue = Options.TicketDataFormat.Protect(ticket);
 
-            if (model.Properties.IsPersistent)
+            var cookieOptions = BuildCookieOptions();
+            if (ticket.Properties.IsPersistent && _renewExpiresUtc.HasValue)
             {
-                cookieOptions.Expires = _renewExpiresUtc.ToUniversalTime().DateTime;
+                cookieOptions.Expires = _renewExpiresUtc.Value.ToUniversalTime().DateTime;
             }
 
             Options.CookieManager.AppendResponseCookie(
@@ -160,17 +170,9 @@ namespace Microsoft.AspNet.Authentication.Cookies
                 cookieValue,
                 cookieOptions);
 
-            Response.Headers.Set(
+            Response.Headers.Set(HeaderNameCacheControl, HeaderValueNoCache);
-                HeaderNameCacheControl,
+            Response.Headers.Set(HeaderNamePragma, HeaderValueNoCache);
-                HeaderValueNoCache);
+            Response.Headers.Set(HeaderNameExpires, HeaderValueMinusOne);
-
-            Response.Headers.Set(
-                HeaderNamePragma,
-                HeaderValueNoCache);
-
-            Response.Headers.Set(
-                HeaderNameExpires,
-                HeaderValueMinusOne);
         }
 
         protected override async Task FinishResponseAsync()
@@ -181,15 +183,15 @@ namespace Microsoft.AspNet.Authentication.Cookies
                 return;
             }
 
-            var model = await AuthenticateAsync();
+            var ticket = await AuthenticateOnceAsync();
             try
             {
-                await ApplyCookie(model);
+                await ApplyCookie(ticket);
             }
             catch (Exception exception)
             {
                 var exceptionContext = new CookieExceptionContext(Context, Options,
-                    CookieExceptionContext.ExceptionLocation.ApplyResponseGrant, exception, model);
+                    CookieExceptionContext.ExceptionLocation.ApplyResponseGrant, exception, ticket);
                 Options.Notifications.Exception(exceptionContext);
                 if (exceptionContext.Rethrow)
                 {
@@ -286,7 +288,7 @@ namespace Microsoft.AspNet.Authentication.Cookies
                 {
                     var query = Request.Query;
                     var redirectUri = query.Get(Options.ReturnUrlParameter);
-                    if (!string.IsNullOrWhiteSpace(redirectUri)
+                    if (!string.IsNullOrEmpty(redirectUri)
                         && IsHostRelative(redirectUri))
                     {
                         var redirectContext = new CookieApplyRedirectContext(Context, Options, redirectUri);
@@ -348,7 +350,7 @@ namespace Microsoft.AspNet.Authentication.Cookies
                 {
                     var query = Request.Query;
                     var redirectUri = query.Get(Options.ReturnUrlParameter);
-                    if (!string.IsNullOrWhiteSpace(redirectUri)
+                    if (!string.IsNullOrEmpty(redirectUri)
                         && IsHostRelative(redirectUri))
                     {
                         var redirectContext = new CookieApplyRedirectContext(Context, Options, redirectUri);
@@ -427,7 +429,7 @@ namespace Microsoft.AspNet.Authentication.Cookies
             var redirectUri = new AuthenticationProperties(context.Properties).RedirectUri;
             try
             {
-                if (string.IsNullOrWhiteSpace(redirectUri))
+                if (string.IsNullOrEmpty(redirectUri))
                 {
                     redirectUri =
                         Request.PathBase +

src/Microsoft.AspNet.Authentication.Cookies/Notifications/CookieValidateIdentityContext.cs

@@ -39,14 +39,19 @@ namespace Microsoft.AspNet.Authentication.Cookies
         /// </summary>
         public AuthenticationProperties Properties { get; private set; }
 
+        /// <summary>
+        /// If true, the cookie will be renewed
+        /// </summary>
+        public bool ShouldRenew { get; set; }
+
         /// <summary>
         /// Called to replace the claims principal. The supplied principal will replace the value of the 
         /// Principal property, which determines the identity of the authenticated request.
         /// </summary>
         /// <param name="identity">The identity used as the replacement</param>
-        public void ReplacePrincipal(IPrincipal principal)
+        public void ReplacePrincipal(ClaimsPrincipal principal)
         {
-            Principal = new ClaimsPrincipal(principal);
+            Principal = principal;
         }
 
         /// <summary>

src/Microsoft.AspNet.Authentication.OAuth/OAuthAuthenticationHandler.cs

@@ -79,7 +79,7 @@ namespace Microsoft.AspNet.Authentication.OAuth
             return context.IsRequestCompleted;
         }
 
-        public override async Task<AuthenticationTicket> AuthenticateAsync()
+        protected override async Task<AuthenticationTicket> AuthenticateAsync()
         {
             AuthenticationProperties properties = null;
             try

src/Microsoft.AspNet.Authentication.OAuthBearer/OAuthBearerAuthenticationHandler.cs

@@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Authentication.OAuthBearer
         /// Searches the 'Authorization' header for a 'Bearer' token. If the 'Bearer' token is found, it is validated using <see cref="TokenValidationParameters"/> set in the options.
         /// </summary>
         /// <returns></returns>
-        public override async Task<AuthenticationTicket> AuthenticateAsync()
+        protected override async Task<AuthenticationTicket> AuthenticateAsync()
         {
             string token = null;
             try

src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationHandler.cs

@@ -201,7 +201,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
         /// </summary>
         /// <returns>An <see cref="AuthenticationTicket"/> if successful.</returns>
         /// <remarks>Uses log id's OIDCH-0000 - OIDCH-0025</remarks>
-        public override async Task<AuthenticationTicket> AuthenticateAsync()
+        protected override async Task<AuthenticationTicket> AuthenticateAsync()
         {
             Logger.LogDebug(Resources.OIDCH_0000_AuthenticateCoreAsync, this.GetType());
 

src/Microsoft.AspNet.Authentication.Twitter/TwitterAuthenticationHandler.cs

@@ -44,7 +44,7 @@ namespace Microsoft.AspNet.Authentication.Twitter
             return false;
         }
 
-        public override async Task<AuthenticationTicket> AuthenticateAsync()
+        protected override async Task<AuthenticationTicket> AuthenticateAsync()
         {
             AuthenticationProperties properties = null;
             try

src/Microsoft.AspNet.Authentication/AuthenticationHandler.cs

@@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Authentication
     /// </summary>
     public abstract class AuthenticationHandler : IAuthenticationHandler
     {
+        private Task<AuthenticationTicket> _authenticateTask;
         private bool _finishCalled;
         private AuthenticationOptions _baseOptions;
 
@@ -68,9 +69,10 @@ namespace Microsoft.AspNet.Authentication
 
             Response.OnStarting(OnStartingCallback, this);
 
-            if (BaseOptions.AutomaticAuthentication)
+            // Automatic authentication is the empty scheme
+            if (ShouldHandleScheme(string.Empty))
             {
-                var ticket = await AuthenticateAsync();
+                var ticket = await AuthenticateOnceAsync();
                 if (ticket?.Principal != null)
                 {
                     SecurityHelper.AddUserPrincipal(Context, ticket.Principal);
@@ -155,14 +157,25 @@ namespace Microsoft.AspNet.Authentication
             }
         }
 
+        protected Task<AuthenticationTicket> AuthenticateOnceAsync()
+        {
+            if (_authenticateTask == null)
+            {
+                _authenticateTask = AuthenticateAsync();
+            }
+            return _authenticateTask;
+        }
+
         public async Task AuthenticateAsync(AuthenticateContext context)
         {
             if (ShouldHandleScheme(context.AuthenticationScheme))
             {
-                var ticket = await AuthenticateAsync();
+                // Calling Authenticate more than once should always return the original value. 
+                var ticket = await AuthenticateOnceAsync();
                 if (ticket?.Principal != null)
                 {
                     context.Authenticated(ticket.Principal, ticket.Properties.Items, BaseOptions.Description.Items);
+                    _authenticateTask = Task.FromResult(ticket);
                 }
                 else
                 {
@@ -176,11 +189,7 @@ namespace Microsoft.AspNet.Authentication
             }
         }
 
-        /// <summary>
+        protected abstract Task<AuthenticationTicket> AuthenticateAsync();
-        /// Calling Authenticate more than once should always return the original value. 
-        /// </summary>
-        /// <returns>The ticket data provided by the authentication logic</returns>
-        public abstract Task<AuthenticationTicket> AuthenticateAsync();
 
         public bool ShouldHandleScheme(string authenticationScheme)
         {

src/Microsoft.AspNet.Authentication/DataHandler/Serializer/TicketSerializer.cs

@@ -51,7 +51,7 @@ namespace Microsoft.AspNet.Authentication.DataHandler.Serializer
                 writer.Write(principal.Identities.Count());
                 foreach (var identity in principal.Identities)
                 {
-                    var authenticationType = string.IsNullOrWhiteSpace(identity.AuthenticationType) ? string.Empty : identity.AuthenticationType;
+                    var authenticationType = string.IsNullOrEmpty(identity.AuthenticationType) ? string.Empty : identity.AuthenticationType;
                     writer.Write(authenticationType);
                     WriteWithDefault(writer, identity.NameClaimType, DefaultValues.NameClaimType);
                     WriteWithDefault(writer, identity.RoleClaimType, DefaultValues.RoleClaimType);

test/Microsoft.AspNet.Authentication.Test/AuthenticationHandlerFacts.cs

@@ -61,7 +61,7 @@ namespace Microsoft.AspNet.Authentication
                 Options.AuthenticationScheme = scheme;
             }
 
-            public override Task<AuthenticationTicket> AuthenticateAsync()
+            protected override Task<AuthenticationTicket> AuthenticateAsync()
             {
                 throw new NotImplementedException();
             }
@@ -86,7 +86,7 @@ namespace Microsoft.AspNet.Authentication
                 Options.AutomaticAuthentication = auto;
             }
 
-            public override Task<AuthenticationTicket> AuthenticateAsync()
+            protected override Task<AuthenticationTicket> AuthenticateAsync()
             {
                 throw new NotImplementedException();
             }

test/Microsoft.AspNet.Authentication.Test/Cookies/CookieMiddlewareTests.cs

@@ -341,6 +341,129 @@ namespace Microsoft.AspNet.Authentication.Cookies
             FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe(null);
         }
 
+        [Fact]
+        public async Task ExpiredCookieWithValidatorStillExpired()
+        {
+            var clock = new TestClock();
+            var server = CreateServer(options =>
+            {
+                options.SystemClock = clock;
+                options.ExpireTimeSpan = TimeSpan.FromMinutes(10);
+                options.Notifications = new CookieAuthenticationNotifications
+                {
+                    OnValidatePrincipal = ctx =>
+                    {
+                        ctx.ShouldRenew = true;
+                        return Task.FromResult(0);
+                    }
+                };
+            },
+            context =>
+                context.Authentication.SignInAsync("Cookies",
+                    new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity("Alice", "Cookies")))));
+
+            var transaction1 = await SendAsync(server, "http://example.com/testpath");
+
+            clock.Add(TimeSpan.FromMinutes(11));
+
+            var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction2.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe(null);
+        }
+
+        [Fact]
+        public async Task CookieCanBeRenewedByValidator()
+        {
+            var clock = new TestClock();
+            var server = CreateServer(options =>
+            {
+                options.SystemClock = clock;
+                options.ExpireTimeSpan = TimeSpan.FromMinutes(10);
+                options.SlidingExpiration = false;
+                options.Notifications = new CookieAuthenticationNotifications
+                {
+                    OnValidatePrincipal = ctx =>
+                    {
+                        ctx.ShouldRenew = true;
+                        return Task.FromResult(0);
+                    }
+                };
+            },
+            context =>
+                context.Authentication.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);
+            transaction2.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
+
+            clock.Add(TimeSpan.FromMinutes(5));
+
+            var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction2.CookieNameValue);
+            transaction3.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
+
+            clock.Add(TimeSpan.FromMinutes(6));
+
+            var transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction4.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe(null);
+
+            clock.Add(TimeSpan.FromMinutes(5));
+
+            var transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction2.CookieNameValue);
+            transaction5.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe(null);
+        }
+
+        [Fact]
+        public async Task CookieCanBeRenewedByValidatorWithSlidingExpiry()
+        {
+            var clock = new TestClock();
+            var server = CreateServer(options =>
+            {
+                options.SystemClock = clock;
+                options.ExpireTimeSpan = TimeSpan.FromMinutes(10);
+                options.Notifications = new CookieAuthenticationNotifications
+                {
+                    OnValidatePrincipal = ctx =>
+                    {
+                        ctx.ShouldRenew = true;
+                        return Task.FromResult(0);
+                    }
+                };
+            },
+            context =>
+                context.Authentication.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);
+            transaction2.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
+
+            clock.Add(TimeSpan.FromMinutes(5));
+
+            var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction2.CookieNameValue);
+            transaction3.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
+
+            clock.Add(TimeSpan.FromMinutes(6));
+
+            var transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction3.CookieNameValue);
+            transaction4.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice");
+
+            clock.Add(TimeSpan.FromMinutes(11));
+
+            var transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue);
+            transaction5.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe(null);
+        }
+
         [Fact]
         public async Task CookieExpirationCanBeOverridenInEvent()
         {
@@ -362,19 +485,18 @@ namespace Microsoft.AspNet.Authentication.Cookies
             var transaction1 = await SendAsync(server, "http://example.com/testpath");
 
             var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction2.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
 
             clock.Add(TimeSpan.FromMinutes(3));
 
             var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction3.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
 
             clock.Add(TimeSpan.FromMinutes(3));
 
             var transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
-
-            transaction2.SetCookie.ShouldBe(null);
-            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
-            transaction3.SetCookie.ShouldBe(null);
-            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
             transaction4.SetCookie.ShouldBe(null);
             FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe(null);
         }
@@ -393,26 +515,25 @@ namespace Microsoft.AspNet.Authentication.Cookies
             var transaction1 = await SendAsync(server, "http://example.com/testpath");
 
             var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction2.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
 
             clock.Add(TimeSpan.FromMinutes(4));
 
             var transaction3 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue);
+            transaction3.SetCookie.ShouldBe(null);
+            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
 
             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);
+            transaction4.SetCookie.ShouldNotBe(null);
+            FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice");
 
             clock.Add(TimeSpan.FromMinutes(4));
 
             Transaction transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue);
-
-            transaction2.SetCookie.ShouldBe(null);
-            FindClaimValue(transaction2, ClaimTypes.Name).ShouldBe("Alice");
-            transaction3.SetCookie.ShouldBe(null);
-            FindClaimValue(transaction3, ClaimTypes.Name).ShouldBe("Alice");
-            transaction4.SetCookie.ShouldNotBe(null);
-            FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice");
             transaction5.SetCookie.ShouldBe(null);
             FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe("Alice");
         }
@@ -427,10 +548,8 @@ namespace Microsoft.AspNet.Authentication.Cookies
             });
 
             var transaction = await SendAsync(server, "http://example.com/protected", ajaxRequest: true);
-
             transaction.Response.StatusCode.ShouldBe(HttpStatusCode.OK);
             var responded = transaction.Response.Headers.GetValues("X-Responded-JSON");
-
             responded.Count().ShouldBe(1);
             responded.Single().ShouldContain("\"location\"");
         }

test/Microsoft.AspNet.Authentication.Test/DataHandler/Encoder/TicketSerializerTests.cs

@@ -52,6 +52,5 @@ namespace Microsoft.AspNet.Authentication.DataHandler.Encoder
                 readTicket.AuthenticationScheme.ShouldBe("Hello");
             }
         }
-
     }
 }