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.

Sign up for GitHub

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

Jump to bottom

Same site cookie fix #261

Merged
merged 8 commits into from
Jan 20, 2020
Merged

Same site cookie fix #261

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d9c62a2
update to Core 3.1
Jan 15, 2020
08b13c0
HandleSameSiteCookieCompatibility first idea
Jan 16, 2020
c0e2ad5
Added comments
Jan 16, 2020
ceb0074
SameSite cookie test in other samples
Jan 16, 2020
5380057
Merge remote-tracking branch 'origin/master' into tibre/sameSiteCooki…
Jan 20, 2020
f294557
Applying HandleSameSiteCookieCompatibility to all other samples
Jan 20, 2020
74b26a9
Updating the .NET Core SDK version for .Microsoft.Identity.Web.Test
jmprieur Jan 20, 2020
f03525e
Merge remote-tracking branch 'origin/master' into tibre/sameSiteCooki…
Jan 20, 2020
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
4 changes: 3 additions & 1 deletion 1-WebApp-OIDC/1-1-MyOrg/Startup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ public void ConfigureServices(IServiceCollection services)
{
// This lambda determines whether user consent for non-essential cookies is needed for a given request.
options.CheckConsentNeeded = context => true;
options.MinimumSameSitePolicy = SameSiteMode.None;
options.MinimumSameSitePolicy = SameSiteMode.Unspecified;
// Handling SameSite cookie according to https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1
options.HandleSameSiteCookieCompatibility();
});

// Sign-in users with the Microsoft identity platform
Expand Down
2 changes: 1 addition & 1 deletion 1-WebApp-OIDC/1-1-MyOrg/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<!--<AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>-->
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
Expand Down
2 changes: 1 addition & 1 deletion 1-WebApp-OIDC/1-2-AnyOrg/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 1-WebApp-OIDC/1-3-AnyOrgOrPersonal/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 1-WebApp-OIDC/1-4-Sovereign/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 1-WebApp-OIDC/1-5-B2C/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 2-WebApp-graph-user/2-1-Call-MSGraph/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 2-WebApp-graph-user/2-2-TokenCache/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 2-WebApp-graph-user/2-3-Multi-Tenant/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-99C23405-82C0-4B27-8A15-F3B0744E06BE</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
<RootNamespace>WebApp_OpenIDConnect_DotNet</RootNamespace>
Expand Down
2 changes: 1 addition & 1 deletion 2-WebApp-graph-user/2-4-Sovereign-Call-MSGraph/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 3-WebApp-multi-APIs/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 4-WebApp-your-API/Client/TodoListClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 4-WebApp-your-API/TodoListService/TodoListService.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-TodoListService-03230DB1-5145-408C-A48B-BE3DAFC56C30</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 5-WebApp-AuthZ/5-1-Roles/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion 5-WebApp-AuthZ/5-2-Groups/WebApp-OpenIDConnect-DotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<UserSecretsId>aspnet-WebApp_OpenIDConnect_DotNet-81EA87AD-E64D-4755-A1CC-5EA47F49B5D8</UserSecretsId>
<WebProject_DirectoryAccessLevelKey>0</WebProject_DirectoryAccessLevelKey>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Identity.Web/Microsoft.Identity.Web.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@


<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
</PropertyGroup>


Expand Down
83 changes: 83 additions & 0 deletions Microsoft.Identity.Web/WebAppServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.AzureAD.UI;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Client;
using Microsoft.Identity.Web.Resource;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using System;
using System.Collections.Generic;
using System.Threading.Tasks;

Expand Down Expand Up @@ -100,6 +103,7 @@ public static IServiceCollection AddMicrosoftIdentityPlatformAuthentication(
OpenIdConnectMiddlewareDiagnostics.Subscribe(options.Events);
}
});

return services;
}

Expand Down Expand Up @@ -158,5 +162,84 @@ public static IServiceCollection AddMsal(this IServiceCollection services, IConf
});
return services;
}

/// <summary>
/// Handles SameSite cookie issue according to the https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1.
/// The default list of user-agents that disallow SameSite None, was taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/
/// </summary>
/// <param name="options"></param>
/// <returns></returns>
public static CookiePolicyOptions HandleSameSiteCookieCompatibility(this CookiePolicyOptions options)
{
return HandleSameSiteCookieCompatibility(options, DisallowsSameSiteNone);
}

/// <summary>
/// Handles SameSite cookie issue according to the docs: https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1
/// The default list of user-agents that disallow SameSite None, was taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/
/// </summary>
/// <param name="options"></param>
/// <param name="disallowsSameSiteNone">If you dont want to use the default user-agent list implementation, the method sent in this parameter will be run against the user-agent and if returned true, SameSite value will be set to Unspecified. The default user-agent list used can be found at: https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/</param>
/// <returns></returns>
public static CookiePolicyOptions HandleSameSiteCookieCompatibility(this CookiePolicyOptions options, Func<string, bool> disallowsSameSiteNone)
{
options.MinimumSameSitePolicy = SameSiteMode.Unspecified;
options.OnAppendCookie = cookieContext =>
CheckSameSite(cookieContext.Context, cookieContext.CookieOptions, disallowsSameSiteNone);
options.OnDeleteCookie = cookieContext =>
CheckSameSite(cookieContext.Context, cookieContext.CookieOptions, disallowsSameSiteNone);
return options;
}

private static void CheckSameSite(HttpContext httpContext, CookieOptions options, Func<string, bool> disallowsSameSiteNone)
{
if (options.SameSite == SameSiteMode.None)
{
var userAgent = httpContext.Request.Headers["User-Agent"].ToString();
if (disallowsSameSiteNone(userAgent))
{
options.SameSite = SameSiteMode.Unspecified;
}
}
}

// Method taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/
public static bool DisallowsSameSiteNone(string userAgent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this list covers all the necessary user agents. If there are more browsers, then maybe pulling this list from a config file would be more dynamic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list was suggested by Barry `s team, and he added this comment in his post:

Under testing with the Azure Active Directory team we have found the following checks work for all the common user agents that Azure Active Directory sees that don’t understand the new value.

But it is definitely not a bullet proof code.

For the list in the config file approach, I am challenged about how to represent a nested check in a list. For example, in the current one there is this nested logic:

userAgent.Contains("Macintosh; Intel Mac OS X 10_14") && 
        userAgent.Contains("Version/") && userAgent.Contains("Safari")

What I understood from this check, is to cover strings like Macintosh; Intel Mac OS X 10_14 <anything here> Version/ <anything here> Safari. How could we represent this in a config file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we'd release Microsoft.Identity.Web as a NuGet package we can quickly update it if a new browser is affected.
I'd think that we would not want to oblige every application developer to have to change a config file. @pmaytak (rather upgrade the NuGet package)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok Jean-Marc. Worst case scenario, the new method HandleSameSiteCookieCompatibility has an overload that accepts a custom validator method.

{
// Cover all iOS based browsers here. This includes:
// - Safari on iOS 12 for iPhone, iPod Touch, iPad
// - WkWebview on iOS 12 for iPhone, iPod Touch, iPad
// - Chrome on iOS 12 for iPhone, iPod Touch, iPad
// All of which are broken by SameSite=None, because they use the iOS networking
// stack.
if (userAgent.Contains("CPU iPhone OS 12") ||
userAgent.Contains("iPad; CPU OS 12"))
{
return true;
}

// Cover Mac OS X based browsers that use the Mac OS networking stack.
// This includes:
// - Safari on Mac OS X.
// This does not include:
// - Chrome on Mac OS X
// Because they do not use the Mac OS networking stack.
if (userAgent.Contains("Macintosh; Intel Mac OS X 10_14") &&
userAgent.Contains("Version/") && userAgent.Contains("Safari"))
{
return true;
}

// Cover Chrome 50-69, because some versions are broken by SameSite=None,
// and none in this range require it.
// Note: this covers some pre-Chromium Edge versions,
// but pre-Chromium Edge does not require SameSite=None.
if (userAgent.Contains("Chrome/5") || userAgent.Contains("Chrome/6"))
{
return true;
}

return false;
}
}
}