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

Commit ea747da

Browse files
committed
Feedback
1 parent 4b722bb commit ea747da

File tree

12 files changed

+75
-66
lines changed

12 files changed

+75
-66
lines changed

NuGet.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44
<add key="AspNetCore" value="https://dotnet.myget.org/F/aspnetcore-ci-dev/api/v3/index.json" />
55
<add key="AspNetCoreTools" value="https://dotnet.myget.org/F/aspnetcore-tools/api/v3/index.json" />
66
<add key="NuGet" value="https://api.nuget.org/v3/index.json" />
7+
<add key="temp" value="C:\gh\HttpAbstractions\artifacts\build" />
78
</packageSources>
89
</configuration>

shared/Microsoft.AspNetCore.ChunkingCookieManager.Sources/ChunkingCookieManager.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options)
284284
{
285285
Path = options.Path,
286286
Domain = options.Domain,
287+
SameSite = options.SameSite,
287288
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
288289
});
289290

@@ -297,6 +298,7 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options)
297298
{
298299
Path = options.Path,
299300
Domain = options.Domain,
301+
SameSite = options.SameSite,
300302
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
301303
});
302304
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public CookieAuthenticationOptions()
2222
ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter;
2323
ExpireTimeSpan = TimeSpan.FromDays(14);
2424
SlidingExpiration = true;
25-
CookieSameSite = SameSiteEnforcementMode.None;
25+
CookieSameSite = SameSiteEnforcementMode.Strict;
2626
CookieHttpOnly = true;
2727
CookieSecure = CookieSecurePolicy.SameAsRequest;
2828
Events = new CookieAuthenticationEvents();
@@ -59,7 +59,7 @@ public string CookieName
5959

6060
/// <summary>
6161
/// Determines if the browser should allow the cookie to be attached to same-site or cross-site requests. The
62-
/// default is None, which means the cookie is allowed to be attached to all same-site and cross-site requests.
62+
/// default is Strict, which means the cookie is only allowed to be attached to same-site requests.
6363
/// </summary>
6464
public SameSiteEnforcementMode CookieSameSite { get; set; }
6565

src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,7 @@ private void WriteNonceCookie(string nonce)
899899
new CookieOptions
900900
{
901901
HttpOnly = true,
902+
SameSite = Http.SameSiteEnforcementMode.Lax,
902903
Secure = Request.IsHttps,
903904
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
904905
});
@@ -930,6 +931,7 @@ private string ReadNonceCookie(string nonce)
930931
var cookieOptions = new CookieOptions
931932
{
932933
HttpOnly = true,
934+
SameSite = Http.SameSiteEnforcementMode.Lax,
933935
Secure = Request.IsHttps
934936
};
935937

src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ protected override async Task<AuthenticateResult> HandleRemoteAuthenticateAsync(
8383
var cookieOptions = new CookieOptions
8484
{
8585
HttpOnly = true,
86+
SameSite = SameSiteEnforcementMode.Lax,
8687
Secure = Request.IsHttps
8788
};
8889

@@ -160,6 +161,7 @@ protected override async Task HandleUnauthorizedAsync(ChallengeContext context)
160161
var cookieOptions = new CookieOptions
161162
{
162163
HttpOnly = true,
164+
SameSite = SameSiteEnforcementMode.Lax,
163165
Secure = Request.IsHttps,
164166
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
165167
};

src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties
203203
var cookieOptions = new CookieOptions
204204
{
205205
HttpOnly = true,
206+
SameSite = SameSiteEnforcementMode.Lax,
206207
Secure = Request.IsHttps,
207208
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
208209
};
@@ -242,6 +243,7 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties
242243
var cookieOptions = new CookieOptions
243244
{
244245
HttpOnly = true,
246+
SameSite = SameSiteEnforcementMode.Lax,
245247
Secure = Request.IsHttps
246248
};
247249
Response.Cookies.Delete(cookieName, cookieOptions);

src/Microsoft.AspNetCore.CookiePolicy/CookiePolicyMiddleware.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public IResponseCookies Cookies
7474

7575
private bool PolicyRequiresCookieOptions()
7676
{
77-
return Policy.SameSite != SameSitePolicy.None || Policy.HttpOnly != HttpOnlyPolicy.None || Policy.Secure != CookieSecurePolicy.None;
77+
return Policy.SameSite != MinimumSameSiteStrictnessPolicy.None || Policy.HttpOnly != HttpOnlyPolicy.None || Policy.Secure != CookieSecurePolicy.None;
7878
}
7979

8080
public void Append(string key, string value)
@@ -153,19 +153,19 @@ private void ApplyPolicy(CookieOptions options)
153153
}
154154
switch (Policy.SameSite)
155155
{
156-
case SameSitePolicy.None:
156+
case MinimumSameSiteStrictnessPolicy.None:
157157
break;
158-
case SameSitePolicy.LaxOrStrict:
158+
case MinimumSameSiteStrictnessPolicy.Lax:
159159
if (options.SameSite == SameSiteEnforcementMode.None)
160160
{
161161
options.SameSite = SameSiteEnforcementMode.Lax;
162162
}
163163
break;
164-
case SameSitePolicy.AlwaysStrict:
164+
case MinimumSameSiteStrictnessPolicy.Strict:
165165
options.SameSite = SameSiteEnforcementMode.Strict;
166166
break;
167167
default:
168-
throw new InvalidOperationException();
168+
throw new InvalidOperationException($"Unrecognized {nameof(MinimumSameSiteStrictnessPolicy)} value {Policy.SameSite.ToString()}");
169169
}
170170
switch (Policy.HttpOnly)
171171
{
@@ -175,7 +175,7 @@ private void ApplyPolicy(CookieOptions options)
175175
case HttpOnlyPolicy.None:
176176
break;
177177
default:
178-
throw new InvalidOperationException();
178+
throw new InvalidOperationException($"Unrecognized {nameof(HttpOnlyPolicy)} value {Policy.HttpOnly.ToString()}");
179179
}
180180
}
181181
}

src/Microsoft.AspNetCore.CookiePolicy/CookiePolicyOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public class CookiePolicyOptions
1515
/// <summary>
1616
/// Affects the cookie's same site attribute.
1717
/// </summary>
18-
public SameSitePolicy SameSite { get; set; } = SameSitePolicy.None;
18+
public MinimumSameSiteStrictnessPolicy SameSite { get; set; } = MinimumSameSiteStrictnessPolicy.Strict;
1919

2020
/// <summary>
2121
/// Affects whether cookies must be HttpOnly.

src/Microsoft.AspNetCore.CookiePolicy/SameSitePolicy.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33

44
namespace Microsoft.AspNetCore.CookiePolicy
55
{
6-
public enum SameSitePolicy
6+
public enum MinimumSameSiteStrictnessPolicy
77
{
88
None = 0,
9-
LaxOrStrict,
10-
AlwaysStrict
9+
Lax,
10+
Strict
1111
}
1212
}

test/Microsoft.AspNetCore.Authentication.Test/CookieTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public async Task SignInCausesDefaultCookieToBeCreated()
136136
Assert.StartsWith("TestCookie=", setCookie);
137137
Assert.Contains("; path=/", setCookie);
138138
Assert.Contains("; httponly", setCookie);
139-
Assert.DoesNotContain("; samesite=", setCookie);
139+
Assert.Contains("; samesite=", setCookie);
140140
Assert.DoesNotContain("; expires=", setCookie);
141141
Assert.DoesNotContain("; domain=", setCookie);
142142
Assert.DoesNotContain("; secure", setCookie);

test/Microsoft.AspNetCore.ChunkingCookieManager.Sources.Test/CookieChunkingTests.cs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void AppendLargeCookie_Appended()
1818
new ChunkingCookieManager() { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
1919
var values = context.Response.Headers["Set-Cookie"];
2020
Assert.Equal(1, values.Count);
21-
Assert.Equal("TestCookie=" + testString + "; path=/", values[0]);
21+
Assert.Equal("TestCookie=" + testString + "; path=/; samesite=lax", values[0]);
2222
}
2323

2424
[Fact]
@@ -32,15 +32,15 @@ public void AppendLargeCookieWithLimit_Chunked()
3232
Assert.Equal(9, values.Count);
3333
Assert.Equal<string[]>(new[]
3434
{
35-
"TestCookie=chunks-8; path=/",
36-
"TestCookieC1=abcdefgh; path=/",
37-
"TestCookieC2=ijklmnop; path=/",
38-
"TestCookieC3=qrstuvwx; path=/",
39-
"TestCookieC4=yz012345; path=/",
40-
"TestCookieC5=6789ABCD; path=/",
41-
"TestCookieC6=EFGHIJKL; path=/",
42-
"TestCookieC7=MNOPQRST; path=/",
43-
"TestCookieC8=UVWXYZ; path=/",
35+
"TestCookie=chunks-8; path=/; samesite=lax",
36+
"TestCookieC1=abcdefgh; path=/; samesite=lax",
37+
"TestCookieC2=ijklmnop; path=/; samesite=lax",
38+
"TestCookieC3=qrstuvwx; path=/; samesite=lax",
39+
"TestCookieC4=yz012345; path=/; samesite=lax",
40+
"TestCookieC5=6789ABCD; path=/; samesite=lax",
41+
"TestCookieC6=EFGHIJKL; path=/; samesite=lax",
42+
"TestCookieC7=MNOPQRST; path=/; samesite=lax",
43+
"TestCookieC8=UVWXYZ; path=/; samesite=lax",
4444
}, values);
4545
}
4646

@@ -116,14 +116,14 @@ public void DeleteChunkedCookieWithOptions_AllDeleted()
116116
Assert.Equal(8, cookies.Count);
117117
Assert.Equal(new[]
118118
{
119-
"TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
120-
"TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
121-
"TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
122-
"TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
123-
"TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
124-
"TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
125-
"TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
126-
"TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/",
119+
"TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
120+
"TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
121+
"TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
122+
"TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
123+
"TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
124+
"TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
125+
"TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
126+
"TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax",
127127
}, cookies);
128128
}
129129
}

test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ await RunTest("/secureAlways",
5959
transaction =>
6060
{
6161
Assert.NotNull(transaction.SetCookie);
62-
Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]);
63-
Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]);
64-
Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]);
65-
Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]);
62+
Assert.Equal("A=A; path=/; secure; samesite=strict", transaction.SetCookie[0]);
63+
Assert.Equal("B=B; path=/; secure; samesite=strict", transaction.SetCookie[1]);
64+
Assert.Equal("C=C; path=/; secure; samesite=strict", transaction.SetCookie[2]);
65+
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
6666
}));
6767
}
6868

@@ -79,10 +79,10 @@ await RunTest("/secureNone",
7979
transaction =>
8080
{
8181
Assert.NotNull(transaction.SetCookie);
82-
Assert.Equal("A=A; path=/", transaction.SetCookie[0]);
83-
Assert.Equal("B=B; path=/", transaction.SetCookie[1]);
84-
Assert.Equal("C=C; path=/", transaction.SetCookie[2]);
85-
Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]);
82+
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
83+
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
84+
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
85+
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
8686
}));
8787
}
8888

@@ -99,19 +99,19 @@ await RunTest("/secureSame",
9999
transaction =>
100100
{
101101
Assert.NotNull(transaction.SetCookie);
102-
Assert.Equal("A=A; path=/", transaction.SetCookie[0]);
103-
Assert.Equal("B=B; path=/", transaction.SetCookie[1]);
104-
Assert.Equal("C=C; path=/", transaction.SetCookie[2]);
105-
Assert.Equal("D=D; path=/", transaction.SetCookie[3]);
102+
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
103+
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
104+
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
105+
Assert.Equal("D=D; path=/; samesite=strict", transaction.SetCookie[3]);
106106
}),
107107
new RequestTest("https://example.com/secureSame",
108108
transaction =>
109109
{
110110
Assert.NotNull(transaction.SetCookie);
111-
Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]);
112-
Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]);
113-
Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]);
114-
Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]);
111+
Assert.Equal("A=A; path=/; secure; samesite=strict", transaction.SetCookie[0]);
112+
Assert.Equal("B=B; path=/; secure; samesite=strict", transaction.SetCookie[1]);
113+
Assert.Equal("C=C; path=/; secure; samesite=strict", transaction.SetCookie[2]);
114+
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
115115
}));
116116
}
117117

@@ -128,10 +128,10 @@ await RunTest("/httpOnlyAlways",
128128
transaction =>
129129
{
130130
Assert.NotNull(transaction.SetCookie);
131-
Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]);
132-
Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]);
133-
Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]);
134-
Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]);
131+
Assert.Equal("A=A; path=/; samesite=strict; httponly", transaction.SetCookie[0]);
132+
Assert.Equal("B=B; path=/; samesite=strict; httponly", transaction.SetCookie[1]);
133+
Assert.Equal("C=C; path=/; samesite=strict; httponly", transaction.SetCookie[2]);
134+
Assert.Equal("D=D; path=/; samesite=strict; httponly", transaction.SetCookie[3]);
135135
}));
136136
}
137137

@@ -148,10 +148,10 @@ await RunTest("/httpOnlyNone",
148148
transaction =>
149149
{
150150
Assert.NotNull(transaction.SetCookie);
151-
Assert.Equal("A=A; path=/", transaction.SetCookie[0]);
152-
Assert.Equal("B=B; path=/", transaction.SetCookie[1]);
153-
Assert.Equal("C=C; path=/", transaction.SetCookie[2]);
154-
Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]);
151+
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
152+
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
153+
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
154+
Assert.Equal("D=D; path=/; samesite=strict; httponly", transaction.SetCookie[3]);
155155
}));
156156
}
157157

@@ -161,7 +161,7 @@ public async Task SameSiteStrictSetsItAlways()
161161
await RunTest("/sameSiteStrict",
162162
new CookiePolicyOptions
163163
{
164-
SameSite = SameSitePolicy.AlwaysStrict
164+
SameSite = MinimumSameSiteStrictnessPolicy.Strict
165165
},
166166
SameSiteCookieAppends,
167167
new RequestTest("http://example.com/sameSiteStrict",
@@ -182,7 +182,7 @@ public async Task SameSiteLaxSetsItAlways()
182182
await RunTest("/sameSiteLax",
183183
new CookiePolicyOptions
184184
{
185-
SameSite = SameSitePolicy.LaxOrStrict
185+
SameSite = MinimumSameSiteStrictnessPolicy.Lax
186186
},
187187
SameSiteCookieAppends,
188188
new RequestTest("http://example.com/sameSiteLax",
@@ -210,10 +210,10 @@ await RunTest("/sameSiteNone",
210210
transaction =>
211211
{
212212
Assert.NotNull(transaction.SetCookie);
213-
Assert.Equal("A=A; path=/", transaction.SetCookie[0]);
214-
Assert.Equal("B=B; path=/", transaction.SetCookie[1]);
215-
Assert.Equal("C=C; path=/", transaction.SetCookie[2]);
216-
Assert.Equal("D=D; path=/; samesite=lax", transaction.SetCookie[3]);
213+
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
214+
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
215+
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
216+
Assert.Equal("D=D; path=/; samesite=strict", transaction.SetCookie[3]);
217217
Assert.Equal("E=E; path=/; samesite=strict", transaction.SetCookie[4]);
218218
}));
219219
}
@@ -242,10 +242,10 @@ public async Task CookiePolicyCanHijackAppend()
242242
var transaction = await server.SendAsync("http://example.com/login");
243243

244244
Assert.NotNull(transaction.SetCookie);
245-
Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[0]);
246-
Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[1]);
247-
Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[2]);
248-
Assert.Equal("Hao=Hao; path=/; secure", transaction.SetCookie[3]);
245+
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[0]);
246+
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[1]);
247+
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[2]);
248+
Assert.Equal("Hao=Hao; path=/; secure; samesite=strict", transaction.SetCookie[3]);
249249
}
250250

251251
[Fact]
@@ -273,7 +273,7 @@ public async Task CookiePolicyCanHijackDelete()
273273

274274
Assert.NotNull(transaction.SetCookie);
275275
Assert.Equal(1, transaction.SetCookie.Count);
276-
Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", transaction.SetCookie[0]);
276+
Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/; samesite=lax", transaction.SetCookie[0]);
277277
}
278278

279279
[Fact]

0 commit comments

Comments
 (0)