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

Update default settings for SameSite #1232

Merged
merged 1 commit into from
May 26, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ project.lock.json
.build/
.testPublish/
/.vs/
.vscode/
global.json
1 change: 1 addition & 0 deletions samples/OpenIdConnect.AzureAdSample/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public static void Main(string[] args)
factory.AddFilter("Console", level => level >= LogLevel.Information);
})
.UseKestrel()
.UseUrls("http://localhost:42023")
.UseContentRoot(Directory.GetCurrentDirectory())
.UseIISIntegration()
.UseStartup<Startup>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ public CookieAuthenticationOptions()
ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter;
ExpireTimeSpan = TimeSpan.FromDays(14);
SlidingExpiration = true;
CookieSameSite = SameSiteMode.Strict;
// To support OAuth authentication, a lax mode is required, see https://github.com/aspnet/Security/issues/1231.
CookieSameSite = SameSiteMode.Lax;
CookieHttpOnly = true;
CookieSecure = CookieSecurePolicy.SameAsRequest;
Events = new CookieAuthenticationEvents();
Expand Down Expand Up @@ -59,7 +60,8 @@ public string CookieName

/// <summary>
/// Determines if the browser should allow the cookie to be attached to same-site or cross-site requests. The
/// default is Strict, which means the cookie is only allowed to be attached to same-site requests.
/// default is Lax, which means the cookie is only allowed to be attached to cross-site requests using safe
/// HTTP methods and same-site requests.
/// </summary>
public SameSiteMode CookieSameSite { get; set; }

Expand All @@ -84,8 +86,8 @@ public string CookieName

/// <summary>
/// Controls how much time the cookie will remain valid from the point it is created. The expiration
/// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
/// even if it is passed to the server after the browser should have purged it
/// information is in the protected cookie ticket. Because of that an expired cookie will be ignored
/// even if it is passed to the server after the browser should have purged it
/// </summary>
public TimeSpan ExpireTimeSpan { get; set; }

Expand All @@ -99,7 +101,7 @@ public string CookieName
/// The LoginPath property informs the handler that it should change an outgoing 401 Unauthorized status
/// code into a 302 redirection onto the given login path. The current url which generated the 401 is added
/// to the LoginPath as a query string parameter named by the ReturnUrlParameter. Once a request to the
/// LoginPath grants a new SignIn identity, the ReturnUrlParameter value is used to redirect the browser back
/// LoginPath grants a new SignIn identity, the ReturnUrlParameter value is used to redirect the browser back
/// to the url which caused the original unauthorized status code.
/// </summary>
public PathString LoginPath { get; set; }
Expand All @@ -117,15 +119,15 @@ public string CookieName

/// <summary>
/// The ReturnUrlParameter determines the name of the query string parameter which is appended by the handler
/// when a 401 Unauthorized status code is changed to a 302 redirect onto the login path. This is also the query
/// string parameter looked for when a request arrives on the login path or logout path, in order to return to the
/// when a 401 Unauthorized status code is changed to a 302 redirect onto the login path. This is also the query
/// string parameter looked for when a request arrives on the login path or logout path, in order to return to the
/// original url after the action is performed.
/// </summary>
public string ReturnUrlParameter { get; set; }

/// <summary>
/// The Provider may be assigned to an instance of an object created by the application at startup time. The handler
/// calls methods on the provider which give the application control at certain points where processing is occurring.
/// calls methods on the provider which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
public new CookieAuthenticationEvents Events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public OpenIdConnectHandler(IOptionsSnapshot<OpenIdConnectOptions> options, ILog
}

/// <summary>
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
protected new OpenIdConnectEvents Events
Expand Down Expand Up @@ -892,7 +892,7 @@ private void WriteNonceCookie(string nonce)
new CookieOptions
{
HttpOnly = true,
SameSite = Http.SameSiteMode.Lax,
SameSite = Http.SameSiteMode.None,
Secure = Request.IsHttps,
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
});
Expand Down Expand Up @@ -924,7 +924,7 @@ private string ReadNonceCookie(string nonce)
var cookieOptions = new CookieOptions
{
HttpOnly = true,
SameSite = Http.SameSiteMode.Lax,
SameSite = Http.SameSiteMode.None,
Secure = Request.IsHttps
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class TwitterHandler : RemoteAuthenticationHandler<TwitterOptions>
private HttpClient Backchannel => Options.Backchannel;

/// <summary>
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
protected new TwitterEvents Events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Microsoft.AspNetCore.Authentication
{
public abstract class RemoteAuthenticationHandler<TOptions> : AuthenticationHandler<TOptions>, IAuthenticationRequestHandler
public abstract class RemoteAuthenticationHandler<TOptions> : AuthenticationHandler<TOptions>, IAuthenticationRequestHandler
where TOptions : RemoteAuthenticationOptions, new()
{
private const string CorrelationPrefix = ".AspNetCore.Correlation.";
Expand All @@ -25,7 +25,7 @@ public abstract class RemoteAuthenticationHandler<TOptions> : AuthenticationHand
protected string SignInScheme => Options.SignInScheme;

/// <summary>
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
protected new RemoteAuthenticationEvents Events
Expand Down Expand Up @@ -203,7 +203,7 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties
var cookieOptions = new CookieOptions
{
HttpOnly = true,
SameSite = SameSiteMode.Lax,
SameSite = SameSiteMode.None,
Secure = Request.IsHttps,
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
};
Expand Down Expand Up @@ -243,7 +243,7 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties
var cookieOptions = new CookieOptions
{
HttpOnly = true,
SameSite = SameSiteMode.Lax,
SameSite = SameSiteMode.None,
Secure = Request.IsHttps
};
Response.Cookies.Delete(cookieName, cookieOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class CookiePolicyOptions
/// <summary>
/// Affects the cookie's same site attribute.
/// </summary>
public SameSiteMode MinimumSameSitePolicy { get; set; } = SameSiteMode.Strict;
public SameSiteMode MinimumSameSitePolicy { get; set; } = SameSiteMode.Lax;

/// <summary>
/// Affects whether cookies must be HttpOnly.
Expand Down
56 changes: 28 additions & 28 deletions test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ await RunTest("/secureAlways",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; secure; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; secure; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; secure; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; secure; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; secure; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; secure; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=lax", transaction.SetCookie[3]);
}));
}

Expand All @@ -79,10 +79,10 @@ await RunTest("/secureNone",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=lax", transaction.SetCookie[3]);
}));
}

Expand All @@ -99,19 +99,19 @@ await RunTest("/secureSame",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=strict", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=lax", transaction.SetCookie[3]);
}),
new RequestTest("https://example.com/secureSame",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; secure; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; secure; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; secure; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=strict", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; secure; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; secure; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; secure; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; secure; samesite=lax", transaction.SetCookie[3]);
}));
}

Expand All @@ -128,10 +128,10 @@ await RunTest("/httpOnlyAlways",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; samesite=strict; httponly", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=strict; httponly", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=strict; httponly", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=strict; httponly", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; samesite=lax; httponly", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=lax; httponly", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=lax; httponly", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=lax; httponly", transaction.SetCookie[3]);
}));
}

Expand All @@ -148,10 +148,10 @@ await RunTest("/httpOnlyNone",
transaction =>
{
Assert.NotNull(transaction.SetCookie);
Assert.Equal("A=A; path=/; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=strict; httponly", transaction.SetCookie[3]);
Assert.Equal("A=A; path=/; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("B=B; path=/; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("C=C; path=/; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("D=D; path=/; samesite=lax; httponly", transaction.SetCookie[3]);
}));
}

Expand Down Expand Up @@ -242,10 +242,10 @@ public async Task CookiePolicyCanHijackAppend()
var transaction = await server.SendAsync("http://example.com/login");

Assert.NotNull(transaction.SetCookie);
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[0]);
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[1]);
Assert.Equal("Hao=Hao; path=/; samesite=strict", transaction.SetCookie[2]);
Assert.Equal("Hao=Hao; path=/; secure; samesite=strict", transaction.SetCookie[3]);
Assert.Equal("Hao=Hao; path=/; samesite=lax", transaction.SetCookie[0]);
Assert.Equal("Hao=Hao; path=/; samesite=lax", transaction.SetCookie[1]);
Assert.Equal("Hao=Hao; path=/; samesite=lax", transaction.SetCookie[2]);
Assert.Equal("Hao=Hao; path=/; secure; samesite=lax", transaction.SetCookie[3]);
}

[Fact]
Expand Down