From a6f39abf9676e5b7fa948b49695c6f19cd0801d3 Mon Sep 17 00:00:00 2001 From: Chris R Date: Thu, 30 Mar 2017 13:46:05 -0700 Subject: [PATCH] #29 Correctly renew persistent cookies when using SessionStore. --- .../CookieAuthenticationHandler.cs | 9 +- .../Cookies/CookieMiddlewareTests.cs | 187 ++++++++++++++++++ 2 files changed, 193 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Owin.Security.Cookies/CookieAuthenticationHandler.cs b/src/Microsoft.Owin.Security.Cookies/CookieAuthenticationHandler.cs index ecabe54d..c4d9e5cd 100644 --- a/src/Microsoft.Owin.Security.Cookies/CookieAuthenticationHandler.cs +++ b/src/Microsoft.Owin.Security.Cookies/CookieAuthenticationHandler.cs @@ -236,8 +236,10 @@ namespace Microsoft.Owin.Security.Cookies } else if (_shouldRenew) { - model.Properties.IssuedUtc = _renewIssuedUtc; - model.Properties.ExpiresUtc = _renewExpiresUtc; + var properties = model.Properties; + + properties.IssuedUtc = _renewIssuedUtc; + properties.ExpiresUtc = _renewExpiresUtc; if (Options.SessionStore != null && _sessionKey != null) { @@ -250,7 +252,8 @@ namespace Microsoft.Owin.Security.Cookies string cookieValue = Options.TicketDataFormat.Protect(model); - if (model.Properties.IsPersistent) + // Check the non-SessionStore properties + if (properties.IsPersistent) { cookieOptions.Expires = _renewExpiresUtc.UtcDateTime; } diff --git a/tests/Microsoft.Owin.Security.Tests/Cookies/CookieMiddlewareTests.cs b/tests/Microsoft.Owin.Security.Tests/Cookies/CookieMiddlewareTests.cs index 4d06e302..c0dd47b0 100644 --- a/tests/Microsoft.Owin.Security.Tests/Cookies/CookieMiddlewareTests.cs +++ b/tests/Microsoft.Owin.Security.Tests/Cookies/CookieMiddlewareTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Net; @@ -74,6 +75,14 @@ namespace Microsoft.Owin.Security.Tests return Task.FromResult(null); } + private Task SignInAsAlicePersistent(IOwinContext context) + { + context.Authentication.SignIn( + new AuthenticationProperties() { IsPersistent = true }, + new ClaimsIdentity(new GenericIdentity("Alice", "Cookies"))); + return Task.FromResult(null); + } + [Fact] public async Task SignInCausesDefaultCookieToBeCreated() { @@ -300,27 +309,171 @@ namespace Microsoft.Owin.Security.Tests Transaction transaction1 = await SendAsync(server, "http://example.com/testpath"); + transaction1.SetCookie.ShouldNotBe(null); + transaction1.SetCookie.ShouldNotContain("Expires"); + Transaction 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)); Transaction 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 Transaction transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue); + transaction4.SetCookie.ShouldNotBe(null); + transaction4.SetCookie.ShouldNotContain("Expires"); + FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice"); + clock.Add(TimeSpan.FromMinutes(4)); Transaction transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue); + transaction5.SetCookie.ShouldBe(null); + FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe("Alice"); + } + + [Fact] + public async Task PersistentCookieIsRenewedWithSlidingExpiration() + { + var clock = new TestClock(); + TestServer server = CreateServer(new CookieAuthenticationOptions + { + SystemClock = clock, + ExpireTimeSpan = TimeSpan.FromMinutes(10), + SlidingExpiration = true, + }, SignInAsAlicePersistent); + + Transaction transaction1 = await SendAsync(server, "http://example.com/testpath"); + + transaction1.SetCookie.ShouldNotBe(null); + transaction1.SetCookie.ShouldContain("Expires"); + + Transaction 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)); + + Transaction 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 + Transaction transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue); + transaction4.SetCookie.ShouldNotBe(null); + transaction4.SetCookie.ShouldContain("Expires"); FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice"); + + clock.Add(TimeSpan.FromMinutes(4)); + + Transaction transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue); + + transaction5.SetCookie.ShouldBe(null); + FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe("Alice"); + } + + [Fact] + public async Task SessionStoreCookieIsRenewedWithSlidingExpiration() + { + var clock = new TestClock(); + TestServer server = CreateServer(new CookieAuthenticationOptions + { + SystemClock = clock, + ExpireTimeSpan = TimeSpan.FromMinutes(10), + SlidingExpiration = true, + SessionStore = new TestSessionStore() + }, SignInAsAlice); + + Transaction transaction1 = await SendAsync(server, "http://example.com/testpath"); + + transaction1.SetCookie.ShouldNotBe(null); + transaction1.SetCookie.ShouldNotContain("Expires"); + + Transaction 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)); + + Transaction 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 + Transaction transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue); + + transaction4.SetCookie.ShouldNotBe(null); + transaction4.SetCookie.ShouldNotContain("Expires"); + FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice"); + + clock.Add(TimeSpan.FromMinutes(4)); + + Transaction transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue); + + transaction5.SetCookie.ShouldBe(null); + FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe("Alice"); + } + + [Fact] + public async Task PersistentSessionStoreCookieIsRenewedWithSlidingExpiration() + { + var clock = new TestClock(); + TestServer server = CreateServer(new CookieAuthenticationOptions + { + SystemClock = clock, + ExpireTimeSpan = TimeSpan.FromMinutes(10), + SlidingExpiration = true, + SessionStore = new TestSessionStore() + }, SignInAsAlicePersistent); + + Transaction transaction1 = await SendAsync(server, "http://example.com/testpath"); + + transaction1.SetCookie.ShouldNotBe(null); + transaction1.SetCookie.ShouldContain("Expires"); + + Transaction 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)); + + Transaction 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 + Transaction transaction4 = await SendAsync(server, "http://example.com/me/Cookies", transaction1.CookieNameValue); + + transaction4.SetCookie.ShouldNotBe(null); + transaction4.SetCookie.ShouldContain("Expires"); + FindClaimValue(transaction4, ClaimTypes.Name).ShouldBe("Alice"); + + clock.Add(TimeSpan.FromMinutes(4)); + + Transaction transaction5 = await SendAsync(server, "http://example.com/me/Cookies", transaction4.CookieNameValue); + transaction5.SetCookie.ShouldBe(null); FindClaimValue(transaction5, ClaimTypes.Name).ShouldBe("Alice"); } @@ -476,5 +629,39 @@ namespace Microsoft.Owin.Security.Tests public string ResponseText { get; set; } public XElement ResponseElement { get; set; } } + + private class TestSessionStore : IAuthenticationSessionStore + { + IDictionary _store = new Dictionary(StringComparer.Ordinal); + + public Task RemoveAsync(string key) + { + _store.Remove(key); + return Task.FromResult(0); + } + + public Task RenewAsync(string key, AuthenticationTicket ticket) + { + _store[key] = ticket; + return Task.FromResult(0); + } + + public Task RetrieveAsync(string key) + { + AuthenticationTicket ticket; + if (_store.TryGetValue(key, out ticket)) + { + return Task.FromResult(ticket); + } + return Task.FromResult(null); + } + + public Task StoreAsync(AuthenticationTicket ticket) + { + var key = Guid.NewGuid().ToString(); + _store[key] = ticket; + return Task.FromResult(key); + } + } } }