Skip to content

Commit

Permalink
Remove Microsoft.AspNetCore.SuppressSameSiteNone compat switch #14739 (…
Browse files Browse the repository at this point in the history 
…#15092)
  • Loading branch information
@Tratcher
Tratcher committed Oct 22, 2019
1 parent 1189a2c commit 1ba180e
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 158 deletions.
25 changes: 6 additions & 19 deletions src/Http/Headers/src/SetCookieHeaderValue.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ public class SetCookieHeaderValue
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 @@ -42,14 +38,6 @@ 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 @@ -106,7 +94,7 @@ public StringSegment Value

public bool Secure { get; set; }

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

public bool HttpOnly { get; set; }

Expand Down Expand Up @@ -145,7 +133,7 @@ public override string ToString()
}

// Allow for Unspecified (-1) to skip SameSite
if (SameSite == SameSiteMode.None && !SuppressSameSiteNone)
if (SameSite == SameSiteMode.None)
{
sameSite = SameSiteNoneToken;
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
Expand Down Expand Up @@ -275,7 +263,7 @@ public void AppendToStringBuilder(StringBuilder builder)
}

// Allow for Unspecified (-1) to skip SameSite
if (SameSite == SameSiteMode.None && !SuppressSameSiteNone)
if (SameSite == SameSiteMode.None)
{
AppendSegment(builder, SameSiteToken, SameSiteNoneToken);
}
Expand Down Expand Up @@ -478,7 +466,7 @@ private static int GetSetCookieLength(StringSegment input, int startIndex, out S
{
if (!ReadEqualsSign(input, ref offset))
{
result.SameSite = SuppressSameSiteNone ? SameSiteMode.Strict : SameSiteMode.Unspecified;
result.SameSite = SameSiteMode.Unspecified;
}
else
{
Expand All @@ -492,14 +480,13 @@ private static int GetSetCookieLength(StringSegment input, int startIndex, out S
{
result.SameSite = SameSiteMode.Lax;
}
else if (!SuppressSameSiteNone
&& StringSegment.Equals(enforcementMode, SameSiteNoneToken, StringComparison.OrdinalIgnoreCase))
else if (StringSegment.Equals(enforcementMode, SameSiteNoneToken, StringComparison.OrdinalIgnoreCase))
{
result.SameSite = SameSiteMode.None;
}
else
{
result.SameSite = SuppressSameSiteNone ? SameSiteMode.Strict : SameSiteMode.Unspecified;
result.SameSite = SameSiteMode.Unspecified;
}
}
}
Expand Down
98 changes: 0 additions & 98 deletions src/Http/Headers/test/SetCookieHeaderValueTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,28 +313,6 @@ 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 @@ -346,32 +324,6 @@ 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 @@ -382,31 +334,6 @@ 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 @@ -417,31 +344,6 @@ 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
14 changes: 1 addition & 13 deletions src/Http/Http.Abstractions/src/CookieBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,8 @@ 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 @@ -66,7 +54,7 @@ public virtual string Name
/// <remarks>
/// Determines the value that will set on <seealso cref="CookieOptions.SameSite"/>.
/// </remarks>
public virtual SameSiteMode SameSite { get; set; } = SuppressSameSiteNone ? SameSiteMode.None : SameSiteMode.Unspecified;
public virtual SameSiteMode SameSite { get; set; } = SameSiteMode.Unspecified;

/// <summary>
/// The policy that will be used to determine <seealso cref="CookieOptions.Secure"/>.
Expand Down
14 changes: 1 addition & 13 deletions src/Http/Http.Features/src/CookieOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,6 @@ namespace Microsoft.AspNetCore.Http
/// </summary>
public class CookieOptions
{
// 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;

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

/// <summary>
/// Creates a default cookie with a path of '/'.
/// </summary>
Expand Down Expand Up @@ -58,7 +46,7 @@ public CookieOptions()
/// Gets or sets the value for the SameSite attribute of the cookie. The default value is <see cref="SameSiteMode.Unspecified"/>
/// </summary>
/// <returns>The <see cref="SameSiteMode"/> representing the enforcement mode of the cookie.</returns>
public SameSiteMode SameSite { get; set; } = SuppressSameSiteNone ? SameSiteMode.None : SameSiteMode.Unspecified;
public SameSiteMode SameSite { get; set; } = SameSiteMode.Unspecified;

/// <summary>
/// Gets or sets a value that indicates whether a cookie is accessible by client-side script.
Expand Down
14 changes: 1 addition & 13 deletions src/Security/CookiePolicy/src/CookiePolicyOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,10 @@ namespace Microsoft.AspNetCore.Builder
/// </summary>
public class CookiePolicyOptions
{
// 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;

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

/// <summary>
/// Affects the cookie's same site attribute.
/// </summary>
public SameSiteMode MinimumSameSitePolicy { get; set; } = SuppressSameSiteNone ? SameSiteMode.None : SameSiteMode.Unspecified;
public SameSiteMode MinimumSameSitePolicy { get; set; } = SameSiteMode.Unspecified;

/// <summary>
/// Affects whether cookies must be HttpOnly.
Expand Down
3 changes: 1 addition & 2 deletions src/Security/CookiePolicy/src/ResponseCookiesWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ public string CreateConsentCookie()
private bool CheckPolicyRequired()
{
return !CanTrack
|| (CookiePolicyOptions.SuppressSameSiteNone && Options.MinimumSameSitePolicy != SameSiteMode.None)
|| (!CookiePolicyOptions.SuppressSameSiteNone && Options.MinimumSameSitePolicy != SameSiteMode.Unspecified)
|| Options.MinimumSameSitePolicy != SameSiteMode.Unspecified
|| Options.HttpOnly != HttpOnlyPolicy.None
|| Options.Secure != CookieSecurePolicy.None;
}
Expand Down

0 comments on commit 1ba180e

Please sign in to comment.