Aspnetcore: Authentication Schemes with spaces in the names no longer work with 5.0 preview 8

Created on 26 Aug 2020 · 6Comments · Source: dotnet/aspnetcore

Describe the bug

After updating aspnet-contrib/AspNet.Security.OAuth.Providers (https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/pull/438) to .NET 5.0 preview 8, the _"Visual Studio Online"_ provider no longer works. Its authentication scheme name is "Visual Studio Online" (note the spaces).

If this is an intentional change then this can be closed, but as Authentication Schemes with spaces in their names have previously been working up until .NET 5.0 preview 7 I'm making the assumption this is a regression.

Removing the spaces from the authentication scheme's name stops the exception from being thrown.

To Reproduce

Checkout https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/pull/438/commits/fb1e5f23c5b30cac85403809d9935470e076d578 and then change the value of AuthenticationScheme here to "Visual Studio Online", then run dotnet test.

Exceptions (if any)

Test Name:  OnCreatingTicket_Is_Raised_By_Handler
Test FullName:  AspNet.Security.OAuth.Providers.Tests.AspNet.Security.OAuth.VisualStudio.VisualStudioTests.OnCreatingTicket_Is_Raised_By_Handler
Test Source:    C:\Coding\aspnet-contrib\AspNet.Security.OAuth.Providers\test\AspNet.Security.OAuth.Providers.Tests\OAuthTests`1.cs : line 138
Test Outcome:   Failed
Test Duration:  0:00:00

Test Name:  OnCreatingTicket_Is_Raised_By_Handler
Test Outcome:   Failed
Result StackTrace:  
at Microsoft.Net.Http.Headers.CookieHeaderValue.CheckNameFormat(StringSegment name, String parameterName)
   at Microsoft.Net.Http.Headers.SetCookieHeaderValue..ctor(StringSegment name, StringSegment value)
   at Microsoft.AspNetCore.Http.ResponseCookies.Append(String key, String value, CookieOptions options)
   at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.GenerateCorrelationId(AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.OAuth.OAuthHandler`1.HandleChallengeAsync(AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.ChallengeAsync(AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.ChallengeAsync(HttpContext context, String scheme, AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.ChallengeAsync(AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.ChallengeAsync(HttpContext context, String scheme, AuthenticationProperties properties)
   at AspNet.Security.OAuth.Infrastructure.ApplicationFactory.<>c__DisplayClass2_0`1.<<ConfigureApplication>b__1>d.MoveNext() in C:\Coding\aspnet-contrib\AspNet.Security.OAuth.Providers\test\AspNet.Security.OAuth.Providers.Tests\Infrastructure\ApplicationFactory.cs:line 117
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.TestHost.ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at AspNet.Security.OAuth.Infrastructure.LoopbackRedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in C:\Coding\aspnet-contrib\AspNet.Security.OAuth.Providers\test\AspNet.Security.OAuth.Providers.Tests\Infrastructure\LoopbackRedirectHandler.cs:line 31
   at System.Net.Http.HttpClient.FinishSendAsync(ValueTask`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts, Boolean buffered, Boolean async, CancellationToken callerToken, Int64 timeoutTime)
   at AspNet.Security.OAuth.OAuthTests`1.AuthenticateUserAsync(WebApplicationFactory`1 server) in C:\Coding\aspnet-contrib\AspNet.Security.OAuth.Providers\test\AspNet.Security.OAuth.Providers.Tests\OAuthTests`1.cs:line 187
   at AspNet.Security.OAuth.OAuthTests`1.OnCreatingTicket_Is_Raised_By_Handler() in C:\Coding\aspnet-contrib\AspNet.Security.OAuth.Providers\test\AspNet.Security.OAuth.Providers.Tests\OAuthTests`1.cs:line 159
--- End of stack trace from previous location ---
Result Message: System.ArgumentException : Invalid cookie name: .AspNetCore.Correlation.Visual Studio Online.TZN9CzHIl8h7Gl7p0EGfLQsctlBmmQ_5JJAqjGaSdxQ (Parameter 'value')
Result StandardOutput:  
[2020-08-26 06:57:25Z] info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[63]
      User profile is available. Using 'C:\Users\martin.costello\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest.
[2020-08-26 06:57:25Z] info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost/me - -
[2020-08-26 06:57:25Z] info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint '/me HTTP: GET'
[2020-08-26 06:57:25Z] info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1]
      Executed endpoint '/me HTTP: GET'
[2020-08-26 06:57:25Z] info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/1.1 GET http://localhost/me - - - 200 - - 2.4439ms

Further technical details

dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.100-preview.8.20417.9
 Commit:    fc62663a35

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19041
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.8.20417.9\

Host (useful for support):
  Version: 5.0.0-preview.8.20407.11
  Commit:  bf456654f9

.NET SDKs installed:
  2.1.809 [C:\Program Files\dotnet\sdk]
  3.1.104 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]
  3.1.302 [C:\Program Files\dotnet\sdk]
  3.1.303 [C:\Program Files\dotnet\sdk]
  3.1.400 [C:\Program Files\dotnet\sdk]
  3.1.401 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.8.20417.9 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.8.20414.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.8.20407.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.8.20411.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

Done area-security bug

Source

martincostello

All 6 comments

If not intentional, looks like the name would need to be encoded in these two places:

https://github.com/dotnet/aspnetcore/blob/3e1e69eccef4ea54c46c7e706413a0448abbbec9/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs#L203

https://github.com/dotnet/aspnetcore/blob/3e1e69eccef4ea54c46c7e706413a0448abbbec9/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs#L223

martincostello on 26 Aug 2020

This is an unanticipated consequence of https://github.com/dotnet/aspnetcore/issues/23578. I'll look into it.

Tratcher on 26 Aug 2020

👍1

Cookie auth is also likely affected:
https://github.com/dotnet/aspnetcore/blob/3e1e69eccef4ea54c46c7e706413a0448abbbec9/src/Security/Authentication/Cookies/src/PostConfigureCookieAuthenticationOptions.cs#L33

Tratcher on 26 Aug 2020

For the correlation cookie I think we can remove the auth scheme name. We already add a unique id to each cookie name so the scheme name isn't important.

For cookie auth we'll need to encode the scheme name.

Tratcher on 26 Aug 2020

👍1

Note the downlevel patch for this issue used a more conservative mitigation that does not introduce the same regression: https://github.com/dotnet/aspnetcore/pull/24389. We did not want to apply that same mitigation in 5.0 because it has side effects for enumeration of request cookies.

Tratcher on 26 Aug 2020

Encode or replace the spaces with something else?