Skip to content

Commit

Permalink
Re-implement SameSite for 2019 #12125
Browse files Browse the repository at this point in the history
  • Loading branch information
@Tratcher
Tratcher committed Sep 5, 2019
1 parent f7a7c98 commit e2daf35
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 25 deletions.
71 changes: 57 additions & 14 deletions src/Http/Headers/src/SetCookieHeaderValue.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,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 UseSameSite2016Compat;

private const string HttpOnlyToken = "httponly";
private const string SeparatorToken = "; ";
private const string EqualsToken = "=";
Expand All @@ -34,6 +40,14 @@ private static readonly HttpHeaderParser<SetCookieHeaderValue> MultipleValuePars
private StringSegment _name;
private StringSegment _value;

static SetCookieHeaderValue()
{
if (AppContext.TryGetSwitch("Microsoft.Net.Http.Headers.SetCookieHeaderValue.UseSameSite2016Compat", out var enabled))
{
UseSameSite2016Compat = enabled;
}
}

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

public bool Secure { get; set; }

public SameSiteMode SameSite { get; set; }
public SameSiteMode SameSite { get; set; } = UseSameSite2016Compat ? SameSiteMode.None : (SameSiteMode)(-1); // 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;
Expand Down Expand Up @@ -130,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 && !UseSameSite2016Compat)
{
sameSite = SameSiteNoneToken;
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
}
else if (SameSite == SameSiteMode.Lax)
{
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 Down Expand Up @@ -172,7 +197,7 @@ public override string ToString()
AppendSegment(ref sb, SecureToken, null);
}

if (SameSite != SameSiteMode.None)
if (sameSite != null)
{
AppendSegment(ref sb, SameSiteToken, sameSite);
}
Expand Down Expand Up @@ -235,9 +260,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 && !UseSameSite2016Compat)
{
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 @@ -289,7 +323,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 @@ -424,25 +458,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 = UseSameSite2016Compat ? SameSiteMode.Strict : (SameSiteMode)(-1); // 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 (!UseSameSite2016Compat
&& StringSegment.Equals(enforcementMode, SameSiteNoneToken, StringComparison.OrdinalIgnoreCase))
{
result.SameSite = SameSiteMode.None;
}
else
{
result.SameSite = SameSiteMode.Strict;
result.SameSite = UseSameSite2016Compat ? SameSiteMode.Strict : (SameSiteMode)(-1); // Unspecified
}
}
}
Expand Down Expand Up @@ -520,4 +563,4 @@ public override int GetHashCode()
^ HttpOnly.GetHashCode();
}
}
}
}
124 changes: 117 additions & 7 deletions src/Http/Headers/test/SetCookieHeaderValueTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static TheoryData<SetCookieHeaderValue, string> SetCookieHeaderDataSet
{
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<IList<SetCookieHeaderValue>, string[]> ListOfSetCookieH
{
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)(-1) // 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<IList<SetCookieHeaderValue>, string[]> ListOfSetCookieH
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_SameSite2016Compat()
{
SetCookieHeaderValue.UseSameSite2016Compat = true;

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

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

SetCookieHeaderValue.UseSameSite2016Compat = 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_SameSite2016Compat()
{
SetCookieHeaderValue.UseSameSite2016Compat = true;

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

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

SetCookieHeaderValue.UseSameSite2016Compat = 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_SameSite2016Compat()
{
SetCookieHeaderValue.UseSameSite2016Compat = 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.UseSameSite2016Compat = 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_SameSite2016Compat()
{
SetCookieHeaderValue.UseSameSite2016Compat = 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.UseSameSite2016Compat = 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
3 changes: 2 additions & 1 deletion ...urity/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk.Web">
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFrameworks>net461;netcoreapp2.1</TargetFrameworks>
Expand All @@ -21,6 +21,7 @@
<Reference Include="Microsoft.Extensions.FileProviders.Embedded" />
<Reference Include="Microsoft.Extensions.Logging.Console" />
<Reference Include="Microsoft.Extensions.Logging.Debug" />
<Reference Include="Microsoft.Net.Http.Headers" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Security/Authentication/test/CookieTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ public async Task CookieOptionsAlterSetCookieHeader()
Assert.Contains(" path=/foo", setCookie1);
Assert.Contains(" domain=another.com", setCookie1);
Assert.Contains(" secure", setCookie1);
Assert.DoesNotContain(" samesite", setCookie1);
Assert.Contains(" samesite=none", setCookie1);
Assert.Contains(" httponly", setCookie1);

var server2 = CreateServer(o =>
Expand Down
3 changes: 2 additions & 1 deletion src/Security/Authentication/test/Microsoft.AspNetCore.Authentication.Test.csproj
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netcoreapp2.1;net461</TargetFrameworks>
Expand Down Expand Up @@ -39,6 +39,7 @@
<Reference Include="Microsoft.AspNetCore.Authentication.Twitter" />
<Reference Include="Microsoft.AspNetCore.Authentication.WsFederation" />
<Reference Include="Microsoft.AspNetCore.TestHost" />
<Reference Include="Microsoft.Net.Http.Headers" />
</ItemGroup>

</Project>
3 changes: 2 additions & 1 deletion src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ public async Task ChallengeSetsNonceAndStateCookies(OpenIdConnectRedirectBehavio
var server = settings.CreateTestServer();
var transaction = await server.SendAsync(ChallengeEndpoint);

Assert.Contains("samesite=none", transaction.SetCookie.First());
var challengeCookies = SetCookieHeaderValue.ParseList(transaction.SetCookie);
var nonceCookie = challengeCookies.Where(cookie => cookie.Name.StartsWith(OpenIdConnectDefaults.CookieNoncePrefix, StringComparison.Ordinal)).Single();
Assert.True(nonceCookie.Expires.HasValue);
Expand Down Expand Up @@ -613,4 +614,4 @@ public async Task Challenge_HasOverwrittenMaxAgeParaFromBaseAuthenticationProper
Assert.Contains("max_age=1234", res.Headers.Location.Query);
}
}
}
}
Loading

0 comments on commit e2daf35

Please sign in to comment.