Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.1] Re-implement SameSite for 2019 #13776

Merged
merged 5 commits into from Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -316,6 +316,7 @@ public partial class RangeItemHeaderValue
}
public enum SameSiteMode
{
Unspecified = -1,
None = 0,
Lax = 1,
Strict = 2,
Expand Down
4 changes: 3 additions & 1 deletion src/Http/Headers/src/SameSiteMode.cs
Expand Up @@ -5,12 +5,14 @@ namespace Microsoft.Net.Http.Headers
{
/// <summary>
/// Indicates if the client should include a cookie on "same-site" or "cross-site" requests.
/// RFC Draft: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00
/// RFC Draft: https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.1
/// </summary>
// This mirrors Microsoft.AspNetCore.Http.SameSiteMode
public enum SameSiteMode
{
/// <summary>No SameSite field will be set, the client should follow its default cookie policy.</summary>
Unspecified = -1,
/// <summary>Indicates the client should disable same-site restrictions.</summary>
None = 0,
/// <summary>Indicates the client should send the cookie with "same-site" requests, and with "cross-site" top-level navigations.</summary>
Lax,
Expand Down
76 changes: 60 additions & 16 deletions src/Http/Headers/src/SetCookieHeaderValue.cs
Expand Up @@ -20,8 +20,14 @@ public class SetCookieHeaderValue
private const string SecureToken = "secure";
// RFC Draft: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00
private const string SameSiteToken = "samesite";
private static readonly string SameSiteNoneToken = SameSiteMode.None.ToString().ToLower();
private static readonly string SameSiteLaxToken = SameSiteMode.Lax.ToString().ToLower();
private static readonly string SameSiteStrictToken = SameSiteMode.Strict.ToString().ToLower();

// True (old): https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-3.1
// False (new): https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.1
internal static bool SuppressSameSiteNone;

private const string HttpOnlyToken = "httponly";
private const string SeparatorToken = "; ";
private const string EqualsToken = "=";
Expand All @@ -36,6 +42,14 @@ public class SetCookieHeaderValue
private StringSegment _name;
private StringSegment _value;

static SetCookieHeaderValue()
{
if (AppContext.TryGetSwitch("Microsoft.AspNetCore.SuppressSameSiteNone", out var enabled))
{
SuppressSameSiteNone = enabled;
}
}

private SetCookieHeaderValue()
{
// Used by the parser to create a new instance of this type.
Expand Down Expand Up @@ -92,16 +106,17 @@ public StringSegment Value

public bool Secure { get; set; }

public SameSiteMode SameSite { get; set; }
public SameSiteMode SameSite { get; set; } = SuppressSameSiteNone ? SameSiteMode.None : SameSiteMode.Unspecified;

public bool HttpOnly { get; set; }

// name="value"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite={Strict|Lax}; httponly
// name="value"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite={strict|lax|none}; httponly
public override string ToString()
{
var length = _name.Length + EqualsToken.Length + _value.Length;

string maxAge = null;
string sameSite = null;

if (Expires.HasValue)
{
Expand Down Expand Up @@ -129,9 +144,20 @@ public override string ToString()
length += SeparatorToken.Length + SecureToken.Length;
}

if (SameSite != SameSiteMode.None)
// Allow for Unspecified (-1) to skip SameSite
if (SameSite == SameSiteMode.None && !SuppressSameSiteNone)
{
sameSite = SameSiteNoneToken;
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
}
else if (SameSite == SameSiteMode.Lax)
{
var sameSite = SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken;
sameSite = SameSiteLaxToken;
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
}
else if (SameSite == SameSiteMode.Strict)
{
sameSite = SameSiteStrictToken;
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
}

Expand All @@ -140,9 +166,9 @@ public override string ToString()
length += SeparatorToken.Length + HttpOnlyToken.Length;
}

return string.Create(length, (this, maxAge), (span, tuple) =>
return string.Create(length, (this, maxAge, sameSite), (span, tuple) =>
{
var (headerValue, maxAgeValue) = tuple;
var (headerValue, maxAgeValue, sameSite) = tuple;

Append(ref span, headerValue._name);
Append(ref span, EqualsToken);
Expand Down Expand Up @@ -180,9 +206,9 @@ public override string ToString()
AppendSegment(ref span, SecureToken, null);
}

if (headerValue.SameSite != SameSiteMode.None)
if (sameSite != null)
{
AppendSegment(ref span, SameSiteToken, headerValue.SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken);
AppendSegment(ref span, SameSiteToken, sameSite);
}

if (headerValue.HttpOnly)
Expand Down Expand Up @@ -248,9 +274,18 @@ public void AppendToStringBuilder(StringBuilder builder)
AppendSegment(builder, SecureToken, null);
}

if (SameSite != SameSiteMode.None)
// Allow for Unspecified (-1) to skip SameSite
if (SameSite == SameSiteMode.None && !SuppressSameSiteNone)
{
AppendSegment(builder, SameSiteToken, SameSiteNoneToken);
}
else if (SameSite == SameSiteMode.Lax)
{
AppendSegment(builder, SameSiteToken, SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken);
AppendSegment(builder, SameSiteToken, SameSiteLaxToken);
}
else if (SameSite == SameSiteMode.Strict)
{
AppendSegment(builder, SameSiteToken, SameSiteStrictToken);
}

if (HttpOnly)
Expand Down Expand Up @@ -302,7 +337,7 @@ public static bool TryParseStrictList(IList<string> inputs, out IList<SetCookieH
return MultipleValueParser.TryParseStrictValues(inputs, out parsedValues);
}

// name=value; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite={Strict|Lax}; httponly
// name=value; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite={Strict|Lax|None}; httponly
private static int GetSetCookieLength(StringSegment input, int startIndex, out SetCookieHeaderValue parsedValue)
{
Contract.Requires(startIndex >= 0);
Expand Down Expand Up @@ -437,25 +472,34 @@ private static int GetSetCookieLength(StringSegment input, int startIndex, out S
{
result.Secure = true;
}
// samesite-av = "SameSite" / "SameSite=" samesite-value
// samesite-value = "Strict" / "Lax"
// samesite-av = "SameSite=" samesite-value
// samesite-value = "Strict" / "Lax" / "None"
else if (StringSegment.Equals(token, SameSiteToken, StringComparison.OrdinalIgnoreCase))
{
if (!ReadEqualsSign(input, ref offset))
{
result.SameSite = SameSiteMode.Strict;
result.SameSite = SuppressSameSiteNone ? SameSiteMode.Strict : SameSiteMode.Unspecified;
}
else
{
var enforcementMode = ReadToSemicolonOrEnd(input, ref offset);

if (StringSegment.Equals(enforcementMode, SameSiteLaxToken, StringComparison.OrdinalIgnoreCase))
if (StringSegment.Equals(enforcementMode, SameSiteStrictToken, StringComparison.OrdinalIgnoreCase))
{
result.SameSite = SameSiteMode.Strict;
}
else if (StringSegment.Equals(enforcementMode, SameSiteLaxToken, StringComparison.OrdinalIgnoreCase))
{
result.SameSite = SameSiteMode.Lax;
}
else if (!SuppressSameSiteNone
&& StringSegment.Equals(enforcementMode, SameSiteNoneToken, StringComparison.OrdinalIgnoreCase))
{
result.SameSite = SameSiteMode.None;
}
else
{
result.SameSite = SameSiteMode.Strict;
result.SameSite = SuppressSameSiteNone ? SameSiteMode.Strict : SameSiteMode.Unspecified;
}
}
}
Expand Down
124 changes: 117 additions & 7 deletions src/Http/Headers/test/SetCookieHeaderValueTest.cs
Expand Up @@ -57,7 +57,7 @@ public class SetCookieHeaderValueTest
{
SameSite = SameSiteMode.None,
};
dataset.Add(header7, "name7=value7");
dataset.Add(header7, "name7=value7; samesite=none");


return dataset;
Expand Down Expand Up @@ -155,9 +155,20 @@ public static TheoryData<string> InvalidCookieValues
{
SameSite = SameSiteMode.Strict
};
var string6a = "name6=value6; samesite";
var string6b = "name6=value6; samesite=Strict";
var string6c = "name6=value6; samesite=invalid";
var string6 = "name6=value6; samesite=Strict";

var header7 = new SetCookieHeaderValue("name7", "value7")
{
SameSite = SameSiteMode.None
};
var string7 = "name7=value7; samesite=None";

var header8 = new SetCookieHeaderValue("name8", "value8")
{
SameSite = SameSiteMode.Unspecified
};
var string8a = "name8=value8; samesite";
var string8b = "name8=value8; samesite=invalid";

dataset.Add(new[] { header1 }.ToList(), new[] { string1 });
dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, string1 });
Expand All @@ -170,9 +181,10 @@ public static TheoryData<string> InvalidCookieValues
dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string.Join(",", string1, string2, string3, string4) });
dataset.Add(new[] { header5 }.ToList(), new[] { string5a });
dataset.Add(new[] { header5 }.ToList(), new[] { string5b });
dataset.Add(new[] { header6 }.ToList(), new[] { string6a });
dataset.Add(new[] { header6 }.ToList(), new[] { string6b });
dataset.Add(new[] { header6 }.ToList(), new[] { string6c });
dataset.Add(new[] { header6 }.ToList(), new[] { string6 });
dataset.Add(new[] { header7 }.ToList(), new[] { string7 });
dataset.Add(new[] { header8 }.ToList(), new[] { string8a });
dataset.Add(new[] { header8 }.ToList(), new[] { string8b });

return dataset;
}
Expand Down Expand Up @@ -301,6 +313,28 @@ public void SetCookieHeaderValue_ToString(SetCookieHeaderValue input, string exp
Assert.Equal(expectedValue, input.ToString());
}

[Fact]
public void SetCookieHeaderValue_ToString_SameSiteNoneCompat()
{
SetCookieHeaderValue.SuppressSameSiteNone = true;

var input = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};

Assert.Equal("name=value", input.ToString());

SetCookieHeaderValue.SuppressSameSiteNone = false;

var input2 = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};

Assert.Equal("name=value; samesite=none", input2.ToString());
}

[Theory]
[MemberData(nameof(SetCookieHeaderDataSet))]
public void SetCookieHeaderValue_AppendToStringBuilder(SetCookieHeaderValue input, string expectedValue)
Expand All @@ -312,6 +346,32 @@ public void SetCookieHeaderValue_AppendToStringBuilder(SetCookieHeaderValue inpu
Assert.Equal(expectedValue, builder.ToString());
}

[Fact]
public void SetCookieHeaderValue_AppendToStringBuilder_SameSiteNoneCompat()
{
SetCookieHeaderValue.SuppressSameSiteNone = true;

var builder = new StringBuilder();
var input = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};

input.AppendToStringBuilder(builder);
Assert.Equal("name=value", builder.ToString());

SetCookieHeaderValue.SuppressSameSiteNone = false;

var builder2 = new StringBuilder();
var input2 = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};

input2.AppendToStringBuilder(builder2);
Assert.Equal("name=value; samesite=none", builder2.ToString());
}

[Theory]
[MemberData(nameof(SetCookieHeaderDataSet))]
public void SetCookieHeaderValue_Parse_AcceptsValidValues(SetCookieHeaderValue cookie, string expectedValue)
Expand All @@ -322,6 +382,31 @@ public void SetCookieHeaderValue_Parse_AcceptsValidValues(SetCookieHeaderValue c
Assert.Equal(expectedValue, header.ToString());
}

[Fact]
public void SetCookieHeaderValue_Parse_AcceptsValidValues_SameSiteNoneCompat()
{
SetCookieHeaderValue.SuppressSameSiteNone = true;
var header = SetCookieHeaderValue.Parse("name=value; samesite=none");

var cookie = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.Strict,
};

Assert.Equal(cookie, header);
Assert.Equal("name=value; samesite=strict", header.ToString());
SetCookieHeaderValue.SuppressSameSiteNone = false;

var header2 = SetCookieHeaderValue.Parse("name=value; samesite=none");

var cookie2 = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};
Assert.Equal(cookie2, header2);
Assert.Equal("name=value; samesite=none", header2.ToString());
}

[Theory]
[MemberData(nameof(SetCookieHeaderDataSet))]
public void SetCookieHeaderValue_TryParse_AcceptsValidValues(SetCookieHeaderValue cookie, string expectedValue)
Expand All @@ -332,6 +417,31 @@ public void SetCookieHeaderValue_TryParse_AcceptsValidValues(SetCookieHeaderValu
Assert.Equal(expectedValue, header.ToString());
}

[Fact]
public void SetCookieHeaderValue_TryParse_AcceptsValidValues_SameSiteNoneCompat()
{
SetCookieHeaderValue.SuppressSameSiteNone = true;
Assert.True(SetCookieHeaderValue.TryParse("name=value; samesite=none", out var header));
var cookie = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.Strict,
};

Assert.Equal(cookie, header);
Assert.Equal("name=value; samesite=strict", header.ToString());

SetCookieHeaderValue.SuppressSameSiteNone = false;

Assert.True(SetCookieHeaderValue.TryParse("name=value; samesite=none", out var header2));
var cookie2 = new SetCookieHeaderValue("name", "value")
{
SameSite = SameSiteMode.None,
};

Assert.Equal(cookie2, header2);
Assert.Equal("name=value; samesite=none", header2.ToString());
}

[Theory]
[MemberData(nameof(InvalidSetCookieHeaderDataSet))]
public void SetCookieHeaderValue_Parse_RejectsInvalidValues(string value)
Expand Down
16 changes: 14 additions & 2 deletions src/Http/Http.Abstractions/src/CookieBuilder.cs
Expand Up @@ -11,8 +11,20 @@ namespace Microsoft.AspNetCore.Http
/// </summary>
public class CookieBuilder
{
// True (old): https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-3.1
// False (new): https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.1
internal static bool SuppressSameSiteNone;

private string _name;

static CookieBuilder()
{
if (AppContext.TryGetSwitch("Microsoft.AspNetCore.SuppressSameSiteNone", out var enabled))
{
SuppressSameSiteNone = enabled;
}
}

/// <summary>
/// The name of the cookie.
/// </summary>
Expand Down Expand Up @@ -49,12 +61,12 @@ public virtual string Name
public virtual bool HttpOnly { get; set; }

/// <summary>
/// The SameSite attribute of the cookie. The default value is <see cref="SameSiteMode.None"/>
/// The SameSite attribute of the cookie. The default value is <see cref="SameSiteMode.Unspecified"/>
/// </summary>
/// <remarks>
/// Determines the value that will set on <seealso cref="CookieOptions.SameSite"/>.
/// </remarks>
public virtual SameSiteMode SameSite { get; set; } = SameSiteMode.None;
public virtual SameSiteMode SameSite { get; set; } = SuppressSameSiteNone ? SameSiteMode.None : SameSiteMode.Unspecified;

/// <summary>
/// The policy that will be used to determine <seealso cref="CookieOptions.Secure"/>.
Expand Down