Skip to content

Commit 0fc4611

Browse files
TratcherTratcher
authored
Use SameSite everywhere (#308)
1 parent c902bb2 commit 0fc4611

File tree

11 files changed

+184
-12
lines changed

11 files changed

+184
-12
lines changed

‎src/Microsoft.Owin.Host.SystemWeb/SystemWebChunkingCookieManager.cs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
163163
bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
164164
bool pathHasValue = !string.IsNullOrEmpty(options.Path);
165165
bool expiresHasValue = options.Expires.HasValue;
166+
bool sameSiteHasValue = options.SameSite.HasValue && SystemWebCookieManager.IsSameSiteAvailable;
166167

167168
string escapedKey = Uri.EscapeDataString(key);
168169
string prefix = escapedKey + "=";
@@ -173,9 +174,12 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
173174
!pathHasValue ? null : "; path=",
174175
!pathHasValue ? null : options.Path,
175176
!expiresHasValue ? null : "; expires=",
176-
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss ", CultureInfo.InvariantCulture) + "GMT",
177+
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss \\G\\M\\T", CultureInfo.InvariantCulture),
177178
!options.Secure ? null : "; secure",
178-
!options.HttpOnly ? null : "; HttpOnly");
179+
!options.HttpOnly ? null : "; HttpOnly",
180+
!sameSiteHasValue ? null : "; SameSite=",
181+
!sameSiteHasValue ? null : GetStringRepresentationOfSameSite(options.SameSite.Value)
182+
);
179183

180184
value = value ?? string.Empty;
181185
bool quoted = false;
@@ -273,6 +277,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
273277
{
274278
Path = options.Path,
275279
Domain = options.Domain,
280+
HttpOnly = options.HttpOnly,
281+
SameSite = options.SameSite,
282+
Secure = options.Secure,
276283
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
277284
});
278285

@@ -286,6 +293,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
286293
{
287294
Path = options.Path,
288295
Domain = options.Domain,
296+
HttpOnly = options.HttpOnly,
297+
SameSite = options.SameSite,
298+
Secure = options.Secure,
289299
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
290300
});
291301
}
@@ -313,6 +323,14 @@ private static void SetOptions(HttpCookie cookie, CookieOptions options, bool do
313323
{
314324
cookie.HttpOnly = true;
315325
}
326+
327+
if (SystemWebCookieManager.IsSameSiteAvailable)
328+
{
329+
SystemWebCookieManager.SameSiteSetter.Invoke(cookie, new object[]
330+
{
331+
options.SameSite ?? (SameSiteMode)(-1) // Unspecified
332+
});
333+
}
316334
}
317335

318336
private static bool IsQuoted(string value)
@@ -329,5 +347,21 @@ private static string Quote(string value)
329347
{
330348
return '"' + value + '"';
331349
}
350+
351+
private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
352+
{
353+
switch (siteMode)
354+
{
355+
case SameSiteMode.None:
356+
return "None";
357+
case SameSiteMode.Lax:
358+
return "Lax";
359+
case SameSiteMode.Strict:
360+
return "Strict";
361+
default:
362+
throw new ArgumentOutOfRangeException("siteMode",
363+
string.Format(CultureInfo.InvariantCulture, "Unexpected SameSiteMode value: {0}", siteMode));
364+
}
365+
}
332366
}
333367
}

‎src/Microsoft.Owin.Host.SystemWeb/SystemWebCookieManager.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Reflection;
56
using System.Web;
67
using Microsoft.Owin.Infrastructure;
78

@@ -12,6 +13,21 @@ namespace Microsoft.Owin.Host.SystemWeb
1213
/// </summary>
1314
public class SystemWebCookieManager : ICookieManager
1415
{
16+
// .NET 4.7.2, but requries a patch to emit SameSite=None
17+
internal static readonly bool IsSameSiteAvailable;
18+
internal static readonly MethodInfo SameSiteSetter;
19+
20+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1810:InitializeReferenceTypeStaticFieldsInline")]
21+
static SystemWebCookieManager()
22+
{
23+
var systemWeb = typeof(HttpContextBase).Assembly;
24+
IsSameSiteAvailable = systemWeb.GetType("System.Web.SameSiteMode") != null;
25+
if (IsSameSiteAvailable)
26+
{
27+
SameSiteSetter = typeof(HttpCookie).GetProperty("SameSite").SetMethod;
28+
}
29+
}
30+
1531
/// <summary>
1632
/// Creates a new instance of SystemWebCookieManager.
1733
/// </summary>
@@ -104,6 +120,13 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
104120
{
105121
cookie.HttpOnly = true;
106122
}
123+
if (IsSameSiteAvailable)
124+
{
125+
SameSiteSetter.Invoke(cookie, new object[]
126+
{
127+
options.SameSite ?? (SameSiteMode)(-1) // Unspecified
128+
});
129+
}
107130

108131
webContext.Response.AppendCookie(cookie);
109132
}
@@ -133,6 +156,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
133156
{
134157
Path = options.Path,
135158
Domain = options.Domain,
159+
HttpOnly = options.HttpOnly,
160+
Secure = options.Secure,
161+
SameSite = options.SameSite,
136162
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
137163
});
138164
}

‎src/Microsoft.Owin.Security.Cookies/CookieAuthenticationHandler.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ protected override async Task ApplyResponseGrantAsync()
146146
{
147147
Domain = Options.CookieDomain,
148148
HttpOnly = Options.CookieHttpOnly,
149+
SameSite = Options.CookieSameSite,
149150
Path = Options.CookiePath ?? "/",
150151
};
151152
if (Options.CookieSecure == CookieSecureOption.SameAsRequest)

‎src/Microsoft.Owin.Security.Cookies/CookieAuthenticationOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ public string CookieName
6565
/// </summary>
6666
public bool CookieHttpOnly { get; set; }
6767

68+
/// <summary>
69+
/// Determines if the browser should allow the cookie to be sent with requests initiated from other sites.
70+
/// The default is 'null' to exclude the setting and let the browser choose the default behavior.
71+
/// </summary>
72+
public SameSiteMode? CookieSameSite { get; set; }
73+
6874
/// <summary>
6975
/// Determines if the cookie should only be transmitted on HTTPS request. The default is to limit the cookie
7076
/// to HTTPS requests if the page which is doing the SignIn is also HTTPS. If you have an HTTPS sign in page

‎src/Microsoft.Owin.Security.OpenIdConnect/OpenidConnectAuthenticationHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,7 @@ protected virtual void RememberNonce(OpenIdConnectMessage message, string nonce)
637637
Convert.ToBase64String(Encoding.UTF8.GetBytes(Options.StateDataFormat.Protect(properties))),
638638
new CookieOptions
639639
{
640+
SameSite = SameSiteMode.None,
640641
HttpOnly = true,
641642
Secure = Request.IsSecure,
642643
Expires = DateTime.UtcNow + Options.ProtocolValidator.NonceLifetime
@@ -667,6 +668,7 @@ protected virtual string RetrieveNonce(OpenIdConnectMessage message)
667668
{
668669
var cookieOptions = new CookieOptions
669670
{
671+
SameSite = SameSiteMode.None,
670672
HttpOnly = true,
671673
Secure = Request.IsSecure
672674
};

‎src/Microsoft.Owin.Security/Infrastructure/AuthenticationHandler.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ protected void GenerateCorrelationId(AuthenticationProperties properties)
215215

216216
var cookieOptions = new CookieOptions
217217
{
218+
SameSite = SameSiteMode.None,
218219
HttpOnly = true,
219220
Secure = Request.IsSecure
220221
};
@@ -243,6 +244,7 @@ protected void GenerateCorrelationId(ICookieManager cookieManager, Authenticatio
243244

244245
var cookieOptions = new CookieOptions
245246
{
247+
SameSite = SameSiteMode.None,
246248
HttpOnly = true,
247249
Secure = Request.IsSecure
248250
};
@@ -277,6 +279,7 @@ protected bool ValidateCorrelationId(AuthenticationProperties properties, ILogge
277279

278280
var cookieOptions = new CookieOptions
279281
{
282+
SameSite = SameSiteMode.None,
280283
HttpOnly = true,
281284
Secure = Request.IsSecure
282285
};
@@ -332,6 +335,7 @@ protected bool ValidateCorrelationId(ICookieManager cookieManager, Authenticatio
332335

333336
var cookieOptions = new CookieOptions
334337
{
338+
SameSite = SameSiteMode.None,
335339
HttpOnly = true,
336340
Secure = Request.IsSecure
337341
};

‎src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
136136
bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
137137
bool pathHasValue = !string.IsNullOrEmpty(options.Path);
138138
bool expiresHasValue = options.Expires.HasValue;
139+
bool sameSiteHasValue = options.SameSite.HasValue;
139140

140141
string escapedKey = Uri.EscapeDataString(key);
141142
string prefix = escapedKey + "=";
@@ -146,9 +147,11 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
146147
!pathHasValue ? null : "; path=",
147148
!pathHasValue ? null : options.Path,
148149
!expiresHasValue ? null : "; expires=",
149-
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss ", CultureInfo.InvariantCulture) + "GMT",
150+
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss \\G\\M\\T", CultureInfo.InvariantCulture),
150151
!options.Secure ? null : "; secure",
151-
!options.HttpOnly ? null : "; HttpOnly");
152+
!options.HttpOnly ? null : "; HttpOnly",
153+
!sameSiteHasValue ? null : "; SameSite=",
154+
!sameSiteHasValue ? null : GetStringRepresentationOfSameSite(options.SameSite.Value));
152155

153156
value = value ?? string.Empty;
154157
bool quoted = false;
@@ -277,6 +280,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
277280
{
278281
Path = options.Path,
279282
Domain = options.Domain,
283+
HttpOnly = options.HttpOnly,
284+
SameSite = options.SameSite,
285+
Secure = options.Secure,
280286
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
281287
});
282288

@@ -290,6 +296,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
290296
{
291297
Path = options.Path,
292298
Domain = options.Domain,
299+
HttpOnly = options.HttpOnly,
300+
SameSite = options.SameSite,
301+
Secure = options.Secure,
293302
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
294303
});
295304
}
@@ -309,5 +318,21 @@ private static string Quote(string value)
309318
{
310319
return '"' + value + '"';
311320
}
321+
322+
private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
323+
{
324+
switch (siteMode)
325+
{
326+
case SameSiteMode.None:
327+
return "None";
328+
case SameSiteMode.Lax:
329+
return "Lax";
330+
case SameSiteMode.Strict:
331+
return "Strict";
332+
default:
333+
throw new ArgumentOutOfRangeException("siteMode",
334+
string.Format(CultureInfo.InvariantCulture, "Unexpected SameSiteMode value: {0}", siteMode));
335+
}
336+
}
312337
}
313338
}

‎src/Microsoft.Owin/ResponseCookieCollection.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,13 @@ public void Delete(string key, CookieOptions options)
138138
{
139139
Path = options.Path,
140140
Domain = options.Domain,
141+
HttpOnly = options.HttpOnly,
142+
SameSite = options.SameSite,
143+
Secure = options.Secure,
141144
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
142145
});
143146
}
144147

145-
/// <summary>
146-
/// Analogous to ToString() but without boxing so
147-
/// we can save a bit of memory.
148-
/// </summary>
149148
private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
150149
{
151150
switch (siteMode)

‎tests/Katana.Sandbox.WebServer/Katana.Sandbox.WebServer.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
<Compile Include="AspNetAuthSessionStore.cs" />
102102
<Compile Include="InMemoryAuthSessionStore.cs" />
103103
<Compile Include="Properties\AssemblyInfo.cs" />
104+
<Compile Include="SameSiteCookieManager.cs" />
104105
<Compile Include="Startup.cs" />
105106
</ItemGroup>
106107
<ItemGroup>

‎tests/Katana.Sandbox.WebServer/SameSiteCookieManager.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Web;
5+
using Microsoft.Owin;
6+
using Microsoft.Owin.Infrastructure;
7+
8+
namespace Katana.Sandbox.WebServer
9+
{
10+
public class SameSiteCookieManager : ICookieManager
11+
{
12+
private readonly ICookieManager _innerManager;
13+
14+
public SameSiteCookieManager()
15+
: this(new CookieManager())
16+
{
17+
}
18+
19+
public SameSiteCookieManager(ICookieManager innerManager)
20+
{
21+
_innerManager = innerManager;
22+
}
23+
24+
public void AppendResponseCookie(IOwinContext context, string key, string value, CookieOptions options)
25+
{
26+
CheckSameSite(context, options);
27+
_innerManager.AppendResponseCookie(context, key, value, options);
28+
}
29+
30+
public void DeleteCookie(IOwinContext context, string key, CookieOptions options)
31+
{
32+
CheckSameSite(context, options);
33+
_innerManager.DeleteCookie(context, key, options);
34+
}
35+
36+
public string GetRequestCookie(IOwinContext context, string key)
37+
{
38+
return _innerManager.GetRequestCookie(context, key);
39+
}
40+
41+
private void CheckSameSite(IOwinContext context, CookieOptions options)
42+
{
43+
if (DisallowsSameSiteNone(context) && options.SameSite == SameSiteMode.None)
44+
{
45+
// IOS12 and Mac OS X 10.14 treat SameSite=None as SameSite=Strict. Exclude the option instead.
46+
// https://bugs.webkit.org/show_bug.cgi?id=198181
47+
options.SameSite = null;
48+
}
49+
}
50+
51+
// https://myip.ms/view/comp_browsers/8568/Safari_12.html
52+
public static bool DisallowsSameSiteNone(IOwinContext context)
53+
{
54+
// TODO: Use your User Agent library of choice here.
55+
var userAgent = context.Request.Headers["User-Agent"];
56+
return userAgent.Contains("CPU iPhone OS 12") // Also covers iPod touch
57+
|| userAgent.Contains("iPad; CPU OS 12")
58+
// Safari 12 and 13 are both broken on Mojave
59+
|| userAgent.Contains("Macintosh; Intel Mac OS X 10_14");
60+
}
61+
}
62+
}

0 commit comments

Comments
 (0)