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

Commit 4b0ee5d

Browse files
author
Nate McMaster
committed
Add CookieBuilder to CookieAuthenticationOptions and obsolete the duplicated properties
1 parent ff9f145 commit 4b0ee5d

File tree

8 files changed

+174
-153
lines changed

8 files changed

+174
-153
lines changed

Security.sln

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Microsoft Visual Studio Solution File, Format Version 12.00
22
# Visual Studio 15
3-
VisualStudioVersion = 15.0.26507.0
3+
VisualStudioVersion = 15.0.26621.2
44
MinimumVisualStudioVersion = 10.0.40219.1
55
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{4D2B6A51-2F9F-44F5-8131-EA5CAC053652}"
66
EndProject
@@ -59,6 +59,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
5959
build\common.props = build\common.props
6060
build\dependencies.props = build\dependencies.props
6161
build\Key.snk = build\Key.snk
62+
NuGet.config = NuGet.config
6263
build\repo.props = build\repo.props
6364
EndProjectSection
6465
EndProject
@@ -484,4 +485,7 @@ Global
484485
{51563775-C659-4907-9BAF-9995BAB87D01} = {7BF11F3A-60B6-4796-B504-579C67FFBA34}
485486
{58194599-F07D-47A3-9DF2-E21A22C5EF9E} = {4D2B6A51-2F9F-44F5-8131-EA5CAC053652}
486487
EndGlobalSection
488+
GlobalSection(ExtensibilityGlobals) = postSolution
489+
SolutionGuid = {ABF8089E-43D0-4010-84A7-7A9DCFE49357}
490+
EndGlobalSection
487491
EndGlobal

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

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private void RequestRefresh(AuthenticationTicket ticket)
104104

105105
private async Task<AuthenticateResult> ReadCookieTicket()
106106
{
107-
var cookie = Options.CookieManager.GetRequestCookie(Context, Options.CookieName);
107+
var cookie = Options.CookieManager.GetRequestCookie(Context, Options.Cookie.Name);
108108
if (string.IsNullOrEmpty(cookie))
109109
{
110110
return AuthenticateResult.NoResult();
@@ -176,23 +176,15 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
176176

177177
private CookieOptions BuildCookieOptions()
178178
{
179-
var cookieOptions = new CookieOptions
180-
{
181-
Domain = Options.CookieDomain,
182-
SameSite = Options.CookieSameSite,
183-
HttpOnly = Options.CookieHttpOnly,
184-
Path = Options.CookiePath ?? (OriginalPathBase.HasValue ? OriginalPathBase.ToString() : "/"),
185-
};
179+
var cookieOptions = Options.Cookie.Build(Context);
180+
// ignore the 'Expires' value as this will be computed elsewhere
181+
cookieOptions.Expires = null;
186182

187-
if (Options.CookieSecure == CookieSecurePolicy.SameAsRequest)
183+
if (Options.Cookie.Path == null && OriginalPathBase.HasValue)
188184
{
189-
cookieOptions.Secure = Request.IsHttps;
185+
cookieOptions.Path = OriginalPathBase.ToString();
190186
}
191-
else
192-
{
193-
cookieOptions.Secure = Options.CookieSecure == CookieSecurePolicy.Always;
194-
}
195-
187+
196188
return cookieOptions;
197189
}
198190

@@ -239,7 +231,7 @@ protected virtual async Task FinishResponseAsync()
239231

240232
Options.CookieManager.AppendResponseCookie(
241233
Context,
242-
Options.CookieName,
234+
Options.Cookie.Name,
243235
cookieValue,
244236
cookieOptions);
245237

@@ -283,14 +275,14 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper
283275

284276
if (!signInContext.Properties.ExpiresUtc.HasValue)
285277
{
286-
signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.ExpireTimeSpan);
278+
signInContext.Properties.ExpiresUtc = issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
287279
}
288280

289281
await Events.SigningIn(signInContext);
290282

291283
if (signInContext.Properties.IsPersistent)
292284
{
293-
var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.ExpireTimeSpan);
285+
var expiresUtc = signInContext.Properties.ExpiresUtc ?? issuedUtc.Add(Options.Cookie.Expiration ?? default(TimeSpan));
294286
signInContext.CookieOptions.Expires = expiresUtc.ToUniversalTime();
295287
}
296288

@@ -314,7 +306,7 @@ public async virtual Task SignInAsync(ClaimsPrincipal user, AuthenticationProper
314306

315307
Options.CookieManager.AppendResponseCookie(
316308
Context,
317-
Options.CookieName,
309+
Options.Cookie.Name,
318310
cookieValue,
319311
signInContext.CookieOptions);
320312

@@ -359,7 +351,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties)
359351

360352
Options.CookieManager.DeleteCookie(
361353
Context,
362-
Options.CookieName,
354+
Options.Cookie.Name,
363355
context.CookieOptions);
364356

365357
// Only redirect on the logout path

src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationOptions.cs

Lines changed: 111 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,84 +12,150 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
1212
/// </summary>
1313
public class CookieAuthenticationOptions : AuthenticationSchemeOptions
1414
{
15-
private string _cookieName;
15+
private CookieBuilder _cookieBuilder = new AuthCookieBuilder();
1616

1717
/// <summary>
1818
/// Create an instance of the options initialized with the default values
1919
/// </summary>
2020
public CookieAuthenticationOptions()
2121
{
2222
ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter;
23-
ExpireTimeSpan = TimeSpan.FromDays(14);
2423
SlidingExpiration = true;
25-
// To support OAuth authentication, a lax mode is required, see https://github.com/aspnet/Security/issues/1231.
26-
CookieSameSite = SameSiteMode.Lax;
27-
CookieHttpOnly = true;
28-
CookieSecure = CookieSecurePolicy.SameAsRequest;
2924
Events = new CookieAuthenticationEvents();
3025
}
3126

3227
/// <summary>
33-
/// Determines the cookie name used to persist the identity. The default value is ".AspNetCore.Cookies".
28+
/// <para>
29+
/// Determines the settings used to create the cookie.
30+
/// </para>
31+
/// <para>
32+
/// <seealso cref="CookieBuilder.SameSite"/> defaults to <see cref="SameSiteMode.Lax"/>.
33+
/// <seealso cref="CookieBuilder.HttpOnly"/> defaults to <c>true</c>.
34+
/// <seealso cref="CookieBuilder.SecurePolicy"/> defaults to <see cref="CookieSecurePolicy.SameAsRequest"/>.
35+
/// <seealso cref="CookieBuilder.Expiration"/> defaults to 14 days.
36+
/// </para>
37+
/// </summary>
38+
/// <remarks>
39+
/// <para>
40+
/// The default value for cookie name is ".AspNetCore.Cookies".
3441
/// This value should be changed if you change the name of the AuthenticationScheme, especially if your
3542
/// system uses the cookie authentication handler multiple times.
36-
/// </summary>
37-
public string CookieName
43+
/// </para>
44+
/// <para>
45+
/// <seealso cref="CookieBuilder.SameSite"/> determines if the browser should allow the cookie to be attached to same-site or cross-site requests.
46+
/// 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.
47+
/// </para>
48+
/// <para>
49+
/// <seealso cref="CookieBuilder.HttpOnly"/> determines if the browser should allow the cookie to be accessed by client-side javascript.
50+
/// 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.
51+
/// </para>
52+
/// <para>
53+
/// <seealso cref="CookieBuilder.Expiration"/> controls how much time the cookie will remain valid from the point it is created. The expiration
54+
/// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
55+
/// even if it is passed to the server after the browser should have purged it
56+
/// </para>
57+
/// </remarks>
58+
public CookieBuilder Cookie
3859
{
39-
get { return _cookieName; }
40-
set
41-
{
42-
if (value == null)
43-
{
44-
throw new ArgumentNullException(nameof(value));
45-
}
46-
47-
_cookieName = value;
48-
}
60+
get => _cookieBuilder;
61+
set => _cookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
4962
}
5063

5164
/// <summary>
65+
/// <para>
66+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Name"/> on <see cref="Cookie"/>.
67+
/// </para>
68+
/// <para>
69+
/// Determines the cookie name used to persist the identity. The default value is ".AspNetCore.Cookies".
70+
/// This value should be changed if you change the name of the AuthenticationScheme, especially if your
71+
/// system uses the cookie authentication handler multiple times.
72+
/// </para>
73+
/// </summary>
74+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Domain) + ".")]
75+
public string CookieName { get => Cookie.Name; set => Cookie.Name = value; }
76+
77+
/// <summary>
78+
/// <para>
79+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Domain"/> on <see cref="Cookie"/>.
80+
/// </para>
81+
/// <para>
5282
/// Determines the domain used to create the cookie. Is not provided by default.
83+
/// </para>
5384
/// </summary>
54-
public string CookieDomain { get; set; }
85+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Domain) + ".")]
86+
public string CookieDomain { get => Cookie.Domain; set => Cookie.Domain = value; }
5587

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

6199
/// <summary>
100+
/// <para>
101+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.SameSite"/> on <see cref="Cookie"/>.
102+
/// </para>
103+
/// <para>
62104
/// Determines if the browser should allow the cookie to be attached to same-site or cross-site requests. The
63105
/// default is Lax, which means the cookie is only allowed to be attached to cross-site requests using safe
64106
/// HTTP methods and same-site requests.
107+
/// </para>
65108
/// </summary>
66-
public SameSiteMode CookieSameSite { get; set; }
109+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SameSite) + ".")]
110+
public SameSiteMode CookieSameSite { get => Cookie.SameSite; set => Cookie.SameSite = value; }
67111

68112
/// <summary>
113+
/// <para>
114+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.HttpOnly"/> on <see cref="Cookie"/>.
115+
/// </para>
116+
/// <para>
69117
/// Determines if the browser should allow the cookie to be accessed by client-side javascript. The
70118
/// default is true, which means the cookie will only be passed to http requests and is not made available
71119
/// to script on the page.
120+
/// </para>
72121
/// </summary>
73-
public bool CookieHttpOnly { get; set; }
122+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SameSite) + ".")]
123+
public bool CookieHttpOnly { get => Cookie.HttpOnly; set => Cookie.HttpOnly = value; }
74124

75125
/// <summary>
126+
/// <para>
127+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.SecurePolicy"/> on <see cref="Cookie"/>.
128+
/// </para>
129+
/// <para>
76130
/// Determines if the cookie should only be transmitted on HTTPS request. The default is to limit the cookie
77131
/// to HTTPS requests if the page which is doing the SignIn is also HTTPS. If you have an HTTPS sign in page
78132
/// and portions of your site are HTTP you may need to change this value.
133+
/// </para>
79134
/// </summary>
80-
public CookieSecurePolicy CookieSecure { get; set; }
135+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SecurePolicy) + ".")]
136+
public CookieSecurePolicy CookieSecure { get => Cookie.SecurePolicy; set => Cookie.SecurePolicy = value; }
81137

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

87143
/// <summary>
144+
/// <para>
145+
/// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Expiration"/> on <see cref="Cookie"/>.
146+
/// </para>
147+
/// <para>
88148
/// Controls how much time the cookie will remain valid from the point it is created. The expiration
89149
/// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
90150
/// even if it is passed to the server after the browser should have purged it
151+
/// </para>
91152
/// </summary>
92-
public TimeSpan ExpireTimeSpan { get; set; }
153+
[Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Expiration) + ".")]
154+
public TimeSpan ExpireTimeSpan
155+
{
156+
get => Cookie.Expiration ?? default(TimeSpan);
157+
set => Cookie.Expiration = value;
158+
}
93159

94160
/// <summary>
95161
/// The SlidingExpiration is set to true to instruct the handler to re-issue a new cookie with a new
@@ -132,8 +198,8 @@ public string CookieName
132198
/// </summary>
133199
public new CookieAuthenticationEvents Events
134200
{
135-
get { return (CookieAuthenticationEvents)base.Events; }
136-
set { base.Events = value; }
201+
get => (CookieAuthenticationEvents)base.Events;
202+
set => base.Events = value;
137203
}
138204

139205
/// <summary>
@@ -154,5 +220,23 @@ public string CookieName
154220
/// to the client. This can be used to mitigate potential problems with very large identities.
155221
/// </summary>
156222
public ITicketStore SessionStore { get; set; }
223+
224+
private class AuthCookieBuilder : CookieBuilder
225+
{
226+
public AuthCookieBuilder()
227+
{
228+
// To support OAuth authentication, a lax mode is required, see https://github.com/aspnet/Security/issues/1231.
229+
SameSite = SameSiteMode.Lax;
230+
HttpOnly = true;
231+
SecurePolicy = CookieSecurePolicy.SameAsRequest;
232+
Expiration = TimeSpan.FromDays(14);
233+
}
234+
235+
public override string Name
236+
{
237+
get => base.Name;
238+
set => base.Name = value ?? throw new ArgumentNullException(nameof(value));
239+
}
240+
}
157241
}
158242
}

src/Microsoft.AspNetCore.Authentication.Cookies/Microsoft.AspNetCore.Authentication.Cookies.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,8 @@
1919
<ProjectReference Include="..\Microsoft.AspNetCore.Authentication\Microsoft.AspNetCore.Authentication.csproj" />
2020
</ItemGroup>
2121

22+
<ItemGroup>
23+
<Folder Include="Properties\" />
24+
</ItemGroup>
25+
2226
</Project>

src/Microsoft.AspNetCore.Authentication.Cookies/PostConfigureCookieAuthenticationOptions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public void PostConfigure(string name, CookieAuthenticationOptions options)
2828
{
2929
options.DataProtectionProvider = options.DataProtectionProvider ?? _dp;
3030

31-
if (String.IsNullOrEmpty(options.CookieName))
31+
if (string.IsNullOrEmpty(options.Cookie.Name))
3232
{
33-
options.CookieName = CookieAuthenticationDefaults.CookiePrefix + name;
33+
options.Cookie.Name = CookieAuthenticationDefaults.CookiePrefix + name;
3434
}
3535
if (options.TicketDataFormat == null)
3636
{

0 commit comments

Comments
 (0)