Revert obsoleting CookieAuthenticationOptions.ExpireTimeSpan (#1296)

natemcmaster · web-flow · commit bd19ba9533cf · 2017-07-05T15:43:43.000-07:00
- Revert the obsoleting of CookieAuthenticationOptions.ExpireTimeSpan in #1285 - Add test to ensure Cookie.Expiration is ignored
diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs
@@ -270,14 +270,14 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper
 
             if (!signInContext.Properties.ExpiresUtc.HasValue)
             {
-                signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
+                signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.ExpireTimeSpan);
             }
 
             await Events.SigningIn(signInContext);
 
             if (signInContext.Properties.IsPersistent)
             {
-                var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
+                var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.ExpireTimeSpan);
                 signInContext.CookieOptions.Expires = expiresUtc.ToUniversalTime();
             }
 
diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationOptions.cs b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationOptions.cs
@@ -21,14 +21,14 @@ public class CookieAuthenticationOptions : AuthenticationSchemeOptions
             SameSite = SameSiteMode.Lax,
             HttpOnly = true,
             SecurePolicy = CookieSecurePolicy.SameAsRequest,
-            Expiration = TimeSpan.FromDays(14),
         };
 
         /// <summary>
         /// Create an instance of the options initialized with the default values
         /// </summary>
         public CookieAuthenticationOptions()
         {
+            ExpireTimeSpan = TimeSpan.FromDays(14);
             ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter;
             SlidingExpiration = true;
             Events = new CookieAuthenticationEvents();
@@ -42,7 +42,6 @@ public CookieAuthenticationOptions()
         /// <seealso cref="CookieBuilder.SameSite"/> defaults to <see cref="SameSiteMode.Lax"/>.
         /// <seealso cref="CookieBuilder.HttpOnly"/> defaults to <c>true</c>.
         /// <seealso cref="CookieBuilder.SecurePolicy"/> defaults to <see cref="CookieSecurePolicy.SameAsRequest"/>.
-        /// <seealso cref="CookieBuilder.Expiration"/> defaults to 14 days.
         /// </para>
         /// </summary>
         /// <remarks>
@@ -60,9 +59,7 @@ public CookieAuthenticationOptions()
         /// The default is true, which means the cookie will only be passed to http requests and is not made available to script on the page.
         /// </para>
         /// <para>
-        /// <seealso cref="CookieBuilder.Expiration"/> controls how much time the cookie will remain valid from the point it is created. The expiration
-        /// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
-        /// even if it is passed to the server after the browser should have purged it
+        /// <seealso cref="CookieBuilder.Expiration"/> is currently ignored. Use <see cref="ExpireTimeSpan"/> to control lifetime of cookie authentication.
         /// </para>
         /// </remarks>
         public CookieBuilder Cookie
@@ -140,6 +137,19 @@ public CookieBuilder Cookie
         /// </summary>
         public ITicketStore SessionStore { get; set; }
 
+        /// <summary>
+        /// <para>
+        /// Controls how much time the authentication ticket stored in the cookie will remain valid from the point it is created
+        /// The expiration information is stored in the protected cookie ticket. Because of that an expired cookie will be ignored
+        /// even if it is passed to the server after the browser should have purged it.
+        /// </para>
+        /// <para>
+        /// This is separate from the value of <seealso cref="CookieOptions.Expires"/>, which specifies
+        /// how long the browser will keep the cookie.
+        /// </para>
+        /// </summary>
+        public TimeSpan ExpireTimeSpan { get; set; }
+
         #region Obsolete API
         /// <summary>
         /// <para>
@@ -201,23 +211,6 @@ public CookieBuilder Cookie
         /// </summary>
         [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SecurePolicy) + ".")]
         public CookieSecurePolicy CookieSecure { get => Cookie.SecurePolicy; set => Cookie.SecurePolicy = value; }
-
-        /// <summary>
-        /// <para>
-        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Expiration"/> on <see cref="Cookie"/>.
-        /// </para>
-        /// <para>
-        /// Controls how much time the cookie will remain valid from the point it is created. The expiration
-        /// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
-        /// even if it is passed to the server after the browser should have purged it
-        /// </para>
-        /// </summary>
-        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Expiration) + ".")]
-        public TimeSpan ExpireTimeSpan
-        {
-            get => Cookie.Expiration ?? default(TimeSpan);
-            set => Cookie.Expiration = value;
-        }
         #endregion
     }
 }
diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/Microsoft.AspNetCore.Authentication.Cookies.csproj b/src/Microsoft.AspNetCore.Authentication.Cookies/Microsoft.AspNetCore.Authentication.Cookies.csproj
@@ -19,8 +19,4 @@
     <ProjectReference Include="..\Microsoft.AspNetCore.Authentication\Microsoft.AspNetCore.Authentication.csproj" />
   </ItemGroup>
 
-  <ItemGroup>
-    <Folder Include="Properties\" />
-  </ItemGroup>
-
 </Project>
diff --git a/test/Microsoft.AspNetCore.Authentication.Test/CookieTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/CookieTests.cs
@@ -143,6 +143,23 @@ public async Task SignInCausesDefaultCookieToBeCreated()
             Assert.DoesNotContain("; secure", setCookie);
         }
 
+        [Fact]
+        public async Task CookieExpirationOptionIsIgnored()
+        {
+            var server = CreateServerWithServices(s => s.AddAuthentication().AddCookie(o =>
+            {
+                o.Cookie.Name = "TestCookie";
+                // this is currently ignored. Users should set o.ExpireTimeSpan instead
+                o.Cookie.Expiration = TimeSpan.FromDays(10);
+            }), SignInAsAlice);
+
+            var transaction = await SendAsync(server, "http://example.com/testpath");
+
+            var setCookie = transaction.SetCookie;
+            Assert.StartsWith("TestCookie=", setCookie);
+            Assert.DoesNotContain("; expires=", setCookie);
+        }
+
         [Fact]
         public async Task SignInWrongAuthTypeThrows()
         {
@@ -277,7 +294,7 @@ public async Task CookieStopsWorkingAfterExpiration()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
             }, SignInAsAlice);
 
@@ -306,7 +323,7 @@ public async Task CookieExpirationCanBeOverridenInSignin()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
             },
             context =>
@@ -339,7 +356,7 @@ public async Task ExpiredCookieWithValidatorStillExpired()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.Events = new CookieAuthenticationEvents
                 {
                     OnValidatePrincipal = ctx =>
@@ -367,7 +384,7 @@ public async Task CookieCanBeRejectedAndSignedOutByValidator()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
                 o.Events = new CookieAuthenticationEvents
                 {
@@ -395,7 +412,7 @@ public async Task CookieNotRenewedAfterSignOut()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
                 o.Events = new CookieAuthenticationEvents
                 {
@@ -431,7 +448,7 @@ public async Task CookieCanBeRenewedByValidator()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
                 o.Events = new CookieAuthenticationEvents
                 {
@@ -476,7 +493,7 @@ public async Task CookieCanBeRenewedByValidatorWithSlidingExpiry()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.Events = new CookieAuthenticationEvents
                 {
                     OnValidatePrincipal = ctx =>
@@ -520,7 +537,7 @@ public async Task CookieValidatorOnlyCalledOnce()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
                 o.Events = new CookieAuthenticationEvents
                 {
@@ -569,7 +586,7 @@ public async Task ShouldRenewUpdatesIssuedExpiredUtc(bool sliding)
             DateTimeOffset? lastExpiresDate = null;
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = sliding;
                 o.Events = new CookieAuthenticationEvents
                 {
@@ -619,7 +636,7 @@ public async Task CookieExpirationCanBeOverridenInEvent()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = false;
                 o.Events = new CookieAuthenticationEvents()
                 {
@@ -656,7 +673,7 @@ public async Task CookieIsRenewedWithSlidingExpiration()
         {
             var server = CreateServer(o =>
             {
-                o.Cookie.Expiration = TimeSpan.FromMinutes(10);
+                o.ExpireTimeSpan = TimeSpan.FromMinutes(10);
                 o.SlidingExpiration = true;
             },
             SignInAsAlice);

Original file line number	Diff line number	Diff line change
`@@ -270,14 +270,14 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper`
`270`	`270`
`271`	`271`	`if (!signInContext.Properties.ExpiresUtc.HasValue)`
`272`	`272`	`{`
`273`		`- signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));`
	`273`	`+ signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.ExpireTimeSpan);`
`274`	`274`	`}`
`275`	`275`
`276`	`276`	`await Events.SigningIn(signInContext);`
`277`	`277`
`278`	`278`	`if (signInContext.Properties.IsPersistent)`
`279`	`279`	`{`
`280`		`- var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));`
	`280`	`+ var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.ExpireTimeSpan);`
`281`	`281`	`signInContext.CookieOptions.Expires = expiresUtc.ToUniversalTime();`
`282`	`282`	`}`
`283`	`283`
Original file line number	Diff line number	Diff line change
`@@ -143,6 +143,23 @@ public async Task SignInCausesDefaultCookieToBeCreated()`
`143`	`143`	`Assert.DoesNotContain("; secure", setCookie);`
`144`	`144`	`}`
`145`	`145`
	`146`	`+ [Fact]`
	`147`	`+ public async Task CookieExpirationOptionIsIgnored()`
	`148`	`+ {`
	`149`	`+ var server = CreateServerWithServices(s => s.AddAuthentication().AddCookie(o =>`
	`150`	`+ {`
	`151`	`+ o.Cookie.Name = "TestCookie";`
	`152`	`+ // this is currently ignored. Users should set o.ExpireTimeSpan instead`
	`153`	`+ o.Cookie.Expiration = TimeSpan.FromDays(10);`
	`154`	`+ }), SignInAsAlice);`
	`155`	`+`
	`156`	`+ var transaction = await SendAsync(server, "http://example.com/testpath");`
	`157`	`+`
	`158`	`+ var setCookie = transaction.SetCookie;`
	`159`	`+ Assert.StartsWith("TestCookie=", setCookie);`
	`160`	`+ Assert.DoesNotContain("; expires=", setCookie);`
	`161`	`+ }`
	`162`	`+`
`146`	`163`	`[Fact]`
`147`	`164`	`public async Task SignInWrongAuthTypeThrows()`
`148`	`165`	`{`
`@@ -277,7 +294,7 @@ public async Task CookieStopsWorkingAfterExpiration()`
`277`	`294`	`{`
`278`	`295`	`var server = CreateServer(o =>`
`279`	`296`	`{`
`280`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`297`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`281`	`298`	`o.SlidingExpiration = false;`
`282`	`299`	`}, SignInAsAlice);`
`283`	`300`
`@@ -306,7 +323,7 @@ public async Task CookieExpirationCanBeOverridenInSignin()`
`306`	`323`	`{`
`307`	`324`	`var server = CreateServer(o =>`
`308`	`325`	`{`
`309`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`326`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`310`	`327`	`o.SlidingExpiration = false;`
`311`	`328`	`},`
`312`	`329`	`context =>`
`@@ -339,7 +356,7 @@ public async Task ExpiredCookieWithValidatorStillExpired()`
`339`	`356`	`{`
`340`	`357`	`var server = CreateServer(o =>`
`341`	`358`	`{`
`342`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`359`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`343`	`360`	`o.Events = new CookieAuthenticationEvents`
`344`	`361`	`{`
`345`	`362`	`OnValidatePrincipal = ctx =>`
`@@ -367,7 +384,7 @@ public async Task CookieCanBeRejectedAndSignedOutByValidator()`
`367`	`384`	`{`
`368`	`385`	`var server = CreateServer(o =>`
`369`	`386`	`{`
`370`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`387`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`371`	`388`	`o.SlidingExpiration = false;`
`372`	`389`	`o.Events = new CookieAuthenticationEvents`
`373`	`390`	`{`
`@@ -395,7 +412,7 @@ public async Task CookieNotRenewedAfterSignOut()`
`395`	`412`	`{`
`396`	`413`	`var server = CreateServer(o =>`
`397`	`414`	`{`
`398`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`415`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`399`	`416`	`o.SlidingExpiration = false;`
`400`	`417`	`o.Events = new CookieAuthenticationEvents`
`401`	`418`	`{`
`@@ -431,7 +448,7 @@ public async Task CookieCanBeRenewedByValidator()`
`431`	`448`	`{`
`432`	`449`	`var server = CreateServer(o =>`
`433`	`450`	`{`
`434`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`451`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`435`	`452`	`o.SlidingExpiration = false;`
`436`	`453`	`o.Events = new CookieAuthenticationEvents`
`437`	`454`	`{`
`@@ -476,7 +493,7 @@ public async Task CookieCanBeRenewedByValidatorWithSlidingExpiry()`
`476`	`493`	`{`
`477`	`494`	`var server = CreateServer(o =>`
`478`	`495`	`{`
`479`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`496`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`480`	`497`	`o.Events = new CookieAuthenticationEvents`
`481`	`498`	`{`
`482`	`499`	`OnValidatePrincipal = ctx =>`
`@@ -520,7 +537,7 @@ public async Task CookieValidatorOnlyCalledOnce()`
`520`	`537`	`{`
`521`	`538`	`var server = CreateServer(o =>`
`522`	`539`	`{`
`523`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`540`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`524`	`541`	`o.SlidingExpiration = false;`
`525`	`542`	`o.Events = new CookieAuthenticationEvents`
`526`	`543`	`{`
`@@ -569,7 +586,7 @@ public async Task ShouldRenewUpdatesIssuedExpiredUtc(bool sliding)`
`569`	`586`	`DateTimeOffset? lastExpiresDate = null;`
`570`	`587`	`var server = CreateServer(o =>`
`571`	`588`	`{`
`572`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`589`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`573`	`590`	`o.SlidingExpiration = sliding;`
`574`	`591`	`o.Events = new CookieAuthenticationEvents`
`575`	`592`	`{`
`@@ -619,7 +636,7 @@ public async Task CookieExpirationCanBeOverridenInEvent()`
`619`	`636`	`{`
`620`	`637`	`var server = CreateServer(o =>`
`621`	`638`	`{`
`622`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`639`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`623`	`640`	`o.SlidingExpiration = false;`
`624`	`641`	`o.Events = new CookieAuthenticationEvents()`
`625`	`642`	`{`
`@@ -656,7 +673,7 @@ public async Task CookieIsRenewedWithSlidingExpiration()`
`656`	`673`	`{`
`657`	`674`	`var server = CreateServer(o =>`
`658`	`675`	`{`
`659`		`- o.Cookie.Expiration = TimeSpan.FromMinutes(10);`
	`676`	`+ o.ExpireTimeSpan = TimeSpan.FromMinutes(10);`
`660`	`677`	`o.SlidingExpiration = true;`
`661`	`678`	`},`
`662`	`679`	`SignInAsAlice);`