Skip to content

Authentication Schemes with spaces in the names no longer work with 5.0 preview 8 #25266

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.

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

Closed
martincostello opened this issue Aug 26, 2020 · 6 comments
Assignees
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Milestone

Comments

@martincostello
Copy link
Member

martincostello commented Aug 26, 2020

Describe the bug

After updating aspnet-contrib/AspNet.Security.OAuth.Providers (aspnet-contrib/AspNet.Security.OAuth.Providers#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 aspnet-contrib/AspNet.Security.OAuth.Providers@fb1e5f2 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
@martincostello
Copy link
Member Author

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

var cookieName = Options.CorrelationCookie.Name + Scheme.Name + "." + correlationId;

var cookieName = Options.CorrelationCookie.Name + Scheme.Name + "." + correlationId;

@Tratcher Tratcher added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Aug 26, 2020
@Tratcher
Copy link
Member

This is an unanticipated consequence of #23578. I'll look into it.

@Tratcher Tratcher self-assigned this Aug 26, 2020
@Tratcher Tratcher added this to the 5.0.0-rc1 milestone Aug 26, 2020
@Tratcher Tratcher added the bug This issue describes a behavior which is not expected - a bug. label Aug 26, 2020
@Tratcher
Copy link
Member

Cookie auth is also likely affected:

options.Cookie.Name = CookieAuthenticationDefaults.CookiePrefix + name;

@Tratcher
Copy link
Member

Tratcher commented Aug 26, 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
Copy link
Member

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

@blowdart
Copy link
Contributor

Encode or replace the spaces with something else?

Tratcher added a commit to Tratcher/aspnetcore that referenced this issue Aug 26, 2020
Pilchie added a commit that referenced this issue Aug 28, 2020
* Handle auth schemes in cookie names #25266

* With unicode
@Tratcher Tratcher added the Done This issue has been fixed label Aug 28, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Sep 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

3 participants