Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Revert obsoleting CookieAuthenticationOptions.ExpireTimeSpan #1296

Merged
merged 2 commits into from
Jul 5, 2017
Merged

Revert obsoleting CookieAuthenticationOptions.ExpireTimeSpan #1296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77353a6
Revert obsoleting CookieAuthenticationOptions.ExpireTimeSpan
Jul 5, 2017
4061408
Add test to ensure Cookie.Expiration is ignored
Jul 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
37 changes: 15 additions & 22 deletions src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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>
Expand All @@ -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
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
}
}
4 changes: 0 additions & 4 deletions ...soft.AspNetCore.Authentication.Cookies/Microsoft.AspNetCore.Authentication.Cookies.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,4 @@
<ProjectReference Include="..\Microsoft.AspNetCore.Authentication\Microsoft.AspNetCore.Authentication.csproj" />
</ItemGroup>

<ItemGroup>
<Folder Include="Properties\" />
</ItemGroup>

</Project>
39 changes: 28 additions & 11 deletions test/Microsoft.AspNetCore.Authentication.Test/CookieTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);
Expand Down