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

Add CookieBuilder to CookieAuthenticationOptions and obsolete the duplicated properties #1285

Merged
merged 1 commit into from
Jul 5, 2017
Merged

Add CookieBuilder to CookieAuthenticationOptions and obsolete the duplicated properties #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion Security.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.26507.0
VisualStudioVersion = 15.0.26621.2
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{4D2B6A51-2F9F-44F5-8131-EA5CAC053652}"
EndProject
Expand Down Expand Up @@ -59,6 +59,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
build\common.props = build\common.props
build\dependencies.props = build\dependencies.props
build\Key.snk = build\Key.snk
NuGet.config = NuGet.config
build\repo.props = build\repo.props
EndProjectSection
EndProject
Expand Down Expand Up @@ -484,4 +485,7 @@ Global
{51563775-C659-4907-9BAF-9995BAB87D01} = {7BF11F3A-60B6-4796-B504-579C67FFBA34}
{58194599-F07D-47A3-9DF2-E21A22C5EF9E} = {4D2B6A51-2F9F-44F5-8131-EA5CAC053652}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {ABF8089E-43D0-4010-84A7-7A9DCFE49357}
EndGlobalSection
EndGlobal
39 changes: 13 additions & 26 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 @@ -14,9 +14,9 @@

namespace Microsoft.AspNetCore.Authentication.Cookies
{
public class CookieAuthenticationHandler :
AuthenticationHandler<CookieAuthenticationOptions>,
IAuthenticationSignInHandler,
public class CookieAuthenticationHandler :
AuthenticationHandler<CookieAuthenticationOptions>,
IAuthenticationSignInHandler,
IAuthenticationSignOutHandler
{
private const string HeaderValueNoCache = "no-cache";
Expand All @@ -37,7 +37,7 @@ public CookieAuthenticationHandler(IOptionsSnapshot<CookieAuthenticationOptions>
{ }

/// <summary>
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
protected new CookieAuthenticationEvents Events
Expand Down Expand Up @@ -104,7 +104,7 @@ private void RequestRefresh(AuthenticationTicket ticket)

private async Task<AuthenticateResult> ReadCookieTicket()
{
var cookie = Options.CookieManager.GetRequestCookie(Context, Options.CookieName);
var cookie = Options.CookieManager.GetRequestCookie(Context, Options.Cookie.Name);
if (string.IsNullOrEmpty(cookie))
{
return AuthenticateResult.NoResult();
Expand Down Expand Up @@ -176,22 +176,9 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()

private CookieOptions BuildCookieOptions()
{
var cookieOptions = new CookieOptions
{
Domain = Options.CookieDomain,
SameSite = Options.CookieSameSite,
HttpOnly = Options.CookieHttpOnly,
Path = Options.CookiePath ?? (OriginalPathBase.HasValue ? OriginalPathBase.ToString() : "/"),
};

if (Options.CookieSecure == CookieSecurePolicy.SameAsRequest)
{
cookieOptions.Secure = Request.IsHttps;
}
else
{
cookieOptions.Secure = Options.CookieSecure == CookieSecurePolicy.Always;
}
var cookieOptions = Options.Cookie.Build(Context);
// ignore the 'Expires' value as this will be computed elsewhere
cookieOptions.Expires = null;

return cookieOptions;
}
Expand Down Expand Up @@ -239,7 +226,7 @@ protected virtual async Task FinishResponseAsync()

Options.CookieManager.AppendResponseCookie(
Context,
Options.CookieName,
Options.Cookie.Name,
cookieValue,
cookieOptions);

Expand Down Expand Up @@ -283,14 +270,14 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper

if (!signInContext.Properties.ExpiresUtc.HasValue)
{
signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.ExpireTimeSpan);
signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback to default(TimeSpan) doesn't make sense, you end up issuing an already expired cookie.

}

await Events.SigningIn(signInContext);

if (signInContext.Properties.IsPersistent)
{
var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.ExpireTimeSpan);
var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
signInContext.CookieOptions.Expires = expiresUtc.ToUniversalTime();
}

Expand All @@ -314,7 +301,7 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper

Options.CookieManager.AppendResponseCookie(
Context,
Options.CookieName,
Options.Cookie.Name,
cookieValue,
signInContext.CookieOptions);

Expand Down Expand Up @@ -359,7 +346,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties)

Options.CookieManager.DeleteCookie(
Context,
Options.CookieName,
Options.Cookie.Name,
context.CookieOptions);

// Only redirect on the logout path
Expand Down
183 changes: 124 additions & 59 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 @@ -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 Microsoft.AspNetCore.Authentication.Internal;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.Http;

Expand All @@ -12,85 +13,69 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
/// </summary>
public class CookieAuthenticationOptions : AuthenticationSchemeOptions
{
private string _cookieName;
private CookieBuilder _cookieBuilder = new RequestPathBaseCookieBuilder
{
// the default name is configured in PostConfigureCookieAuthenticationOptions

// To support OAuth authentication, a lax mode is required, see https://github.com/aspnet/Security/issues/1231.
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()
{
ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter;
ExpireTimeSpan = TimeSpan.FromDays(14);
SlidingExpiration = true;
// To support OAuth authentication, a lax mode is required, see https://github.com/aspnet/Security/issues/1231.
CookieSameSite = SameSiteMode.Lax;
CookieHttpOnly = true;
CookieSecure = CookieSecurePolicy.SameAsRequest;
Events = new CookieAuthenticationEvents();
}

/// <summary>
/// Determines the cookie name used to persist the identity. The default value is ".AspNetCore.Cookies".
/// <para>
/// Determines the settings used to create the cookie.
/// </para>
/// <para>
/// <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>
/// <para>
/// The default value for cookie name is ".AspNetCore.Cookies".
/// This value should be changed if you change the name of the AuthenticationScheme, especially if your
/// system uses the cookie authentication handler multiple times.
/// </summary>
public string CookieName
/// </para>
/// <para>
/// <seealso cref="CookieBuilder.SameSite"/> determines if the browser should allow the cookie to be attached to same-site or cross-site requests.
/// The default is Lax, which means the cookie is only allowed to be attached to cross-site requests using safe HTTP methods and same-site requests.
/// </para>
/// <para>
/// <seealso cref="CookieBuilder.HttpOnly"/> determines if the browser should allow the cookie to be accessed by client-side javascript.
/// 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
/// </para>
/// </remarks>
public CookieBuilder Cookie
{
get { return _cookieName; }
set
{
if (value == null)
{
throw new ArgumentNullException(nameof(value));
}

_cookieName = value;
}
get => _cookieBuilder;
set => _cookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
}

/// <summary>
/// Determines the domain used to create the cookie. Is not provided by default.
/// </summary>
public string CookieDomain { get; set; }

/// <summary>
/// Determines the path used to create the cookie. The default value is "/" for highest browser compatibility.
/// </summary>
public string CookiePath { get; set; }

/// <summary>
/// Determines if the browser should allow the cookie to be attached to same-site or cross-site requests. The
/// default is Lax, which means the cookie is only allowed to be attached to cross-site requests using safe
/// HTTP methods and same-site requests.
/// </summary>
public SameSiteMode CookieSameSite { get; set; }

/// <summary>
/// Determines if the browser should allow the cookie to be accessed by client-side javascript. 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.
/// </summary>
public bool CookieHttpOnly { get; set; }

/// <summary>
/// Determines if the cookie should only be transmitted on HTTPS request. The default is to limit the cookie
/// to HTTPS requests if the page which is doing the SignIn is also HTTPS. If you have an HTTPS sign in page
/// and portions of your site are HTTP you may need to change this value.
/// </summary>
public CookieSecurePolicy CookieSecure { get; set; }

/// <summary>
/// If set this will be used by the CookieAuthenticationHandler for data protection.
/// </summary>
public IDataProtectionProvider DataProtectionProvider { get; set; }

/// <summary>
/// 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
/// </summary>
public TimeSpan ExpireTimeSpan { get; set; }

/// <summary>
/// The SlidingExpiration is set to true to instruct the handler to re-issue a new cookie with a new
/// expiration time any time it processes a request which is more than halfway through the expiration window.
Expand Down Expand Up @@ -132,8 +117,8 @@ public string CookieName
/// </summary>
public new CookieAuthenticationEvents Events
{
get { return (CookieAuthenticationEvents)base.Events; }
set { base.Events = value; }
get => (CookieAuthenticationEvents)base.Events;
set => base.Events = value;
}

/// <summary>
Expand All @@ -154,5 +139,85 @@ public string CookieName
/// to the client. This can be used to mitigate potential problems with very large identities.
/// </summary>
public ITicketStore SessionStore { get; set; }

#region Obsolete API
/// <summary>
/// <para>
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Name"/> on <see cref="Cookie"/>.
/// </para>
/// <para>
/// Determines the cookie name used to persist the identity. The default value is ".AspNetCore.Cookies".
/// This value should be changed if you change the name of the AuthenticationScheme, especially if your
/// system uses the cookie authentication handler multiple times.
/// </para>
/// </summary>
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Domain) + ".")]
public string CookieName { get => Cookie.Name; set => Cookie.Name = value; }

/// <summary>
/// <para>
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Domain"/> on <see cref="Cookie"/>.
/// </para>
/// <para>
/// Determines the domain used to create the cookie. Is not provided by default.
/// </para>
/// </summary>
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Domain) + ".")]
public string CookieDomain { get => Cookie.Domain; set => Cookie.Domain = value; }

/// <summary>
/// <para>
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Path"/> on <see cref="Cookie"/>.
/// </para>
/// <para>
/// Determines the path used to create the cookie. The default value is "/" for highest browser compatibility.
/// </para>
/// </summary>
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Path) + ".")]
public string CookiePath { get => Cookie.Path; set => Cookie.Path = value; }

/// <summary>
/// <para>
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.HttpOnly"/> on <see cref="Cookie"/>.
/// </para>
/// <para>
/// Determines if the browser should allow the cookie to be accessed by client-side javascript. 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>
/// </summary>
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SameSite) + ".")]
public bool CookieHttpOnly { get => Cookie.HttpOnly; set => Cookie.HttpOnly = value; }

/// <summary>
/// <para>
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.SecurePolicy"/> on <see cref="Cookie"/>.
/// </para>
/// <para>
/// Determines if the cookie should only be transmitted on HTTPS request. The default is to limit the cookie
/// to HTTPS requests if the page which is doing the SignIn is also HTTPS. If you have an HTTPS sign in page
/// and portions of your site are HTTP you may need to change this value.
/// </para>
/// </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: 4 additions & 0 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,4 +19,8 @@
<ProjectReference Include="..\Microsoft.AspNetCore.Authentication\Microsoft.AspNetCore.Authentication.csproj" />
</ItemGroup>

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

</Project>
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Authentication.Cookies/PostConfigureCookieAuthenticationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ public void PostConfigure(string name, CookieAuthenticationOptions options)
{
options.DataProtectionProvider = options.DataProtectionProvider ?? _dp;

if (String.IsNullOrEmpty(options.CookieName))
if (string.IsNullOrEmpty(options.Cookie.Name))
{
options.CookieName = CookieAuthenticationDefaults.CookiePrefix + name;
options.Cookie.Name = CookieAuthenticationDefaults.CookiePrefix + name;
}
if (options.TicketDataFormat == null)
{
Expand Down
Loading