Aspnetcore: UseExceptionHandler middleware can't handle exceptions from UseAuthentication middleware

Created on 7 Aug 2019  路  3Comments  路  Source: dotnet/aspnetcore

Describe the bug

When an exception is thrown from the UseAuthentication middleware it propagates back up the pipeline until caught by the UseExceptionHandler middleware which then re-executes the pipeline using the error path which then gets to the UseAuthentication middleware a second time but because the cached Task from the first call to UseAuthentication is returned the second time around the initial exception on the Task is returned as soon as it is awaited causing UseExceptionHandler to catch it as a secondary exception and then throw the original exception it caught.

To Reproduce

Steps to reproduce the behavior:
Version 2.2.6 of ASP.NET Core
Minimal reproduction https://github.com/AquilaSands/AspNetCoreCookieErrorHandlerBug based on https://github.com/aspnet/AspNetCore.Docs/tree/master/aspnetcore/security/authentication/cookie/samples/2.x/CookieSample

  1. Run the minimal repro
  2. Sign in
  3. Follow the 'Contact (Authentication Required)' link
  4. This will cause a CustomException to be thrown from the custom ValidatePrincipal() method
  5. The custom error page is not shown and you get the generic browser 500 error page

Expected behavior

Expect the custom error page to be shown.

Sample log output

https://github.com/AquilaSands/AspNetCoreCookieErrorHandlerBug/blob/dc50eda4b1dfc0bbc926f00109d57b6f2193499d/log-output.txt

Additional context

The behaviour of the authentication middleware is clearly by design but I think it either needs a way to clear the cached Task or it should avoid caching a faulted Task.

Output of dotnet --info

.NET Core SDK (reflecting any global.json):
Version: 2.2.301
Commit: 70d6be0814

Runtime Environment:
OS Name: Windows
OS Version: 10.0.17763
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\2.2.301\

Host (useful for support):
Version: 2.2.6
Commit: 7dac9b1b51

.NET Core SDKs installed:
2.1.700 [C:\Program Files\dotnet\sdk]
2.1.701 [C:\Program Files\dotnet\sdk]
2.2.300 [C:\Program Files\dotnet\sdk]
2.2.301 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

area-security

Most helpful comment

Indeed, not throwing in the first place is your best option 馃榿. UseExceptionHandler is a best effort to show a friendly error, but it will never be able to handle all errors. We'll see if there's something we can do to address this specific scenario.

All 3 comments

Your repro app makes sense, but could you please describe the original exception scenario where you encountered this? Were you able to work around it? That will help us prioritize this.

Hi, the original scenario uses social authentication providers along with the cookie auth scheme but it is essentially the same as the repro, once a user is authenticated they are signed in with the cookie scheme. The app is multi tenant database per customer so it provides an implementation of the ValidatePrincipal() event to get additional claims for authorization from a tenant db I had a bug in my implementation which caused an exception which triggered this issue.

I haven't found a workaround for this issue other than writing my own code a bit better so it doesn't cause exceptions in the first place 馃槃.

Indeed, not throwing in the first place is your best option 馃榿. UseExceptionHandler is a best effort to show a friendly error, but it will never be able to handle all errors. We'll see if there's something we can do to address this specific scenario.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zorthgo picture zorthgo  路  136Comments

danroth27 picture danroth27  路  130Comments

Trcx528 picture Trcx528  路  85Comments

natemcmaster picture natemcmaster  路  213Comments

mkArtakMSFT picture mkArtakMSFT  路  89Comments