Skip to content

IIS (out-of-proc) calls AuthenticateAsync for all requests at the start of the pipeline #7750

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
fractionalJoe opened this issue Feb 20, 2019 · 8 comments
Assignees
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions breaking-change This issue / pr will introduce a breaking change, when resolved / merged. bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed feature-iis Includes: IIS, ANCM

Comments

@fractionalJoe
Copy link

fractionalJoe commented Feb 20, 2019

When implementing a ClaimsTransformation that attempts to access HttpContext.Session, an error "Session has not been configured for this application or request." is raised by Kestrel. This is related to #3805 and this code appears to be the culprit as it invokes authentication middleware before UseSession is called in the Startup.Configure method. I can find no way to check if Session has been been configured without a try/catch block.

To Reproduce

  1. Using IIS 10 and ASP.Net Core 2.2. App must be configured to run Out-of-process for this issue to occur.
  2. I have created a repo @ https://github.com/fractionalJoe/TestIisIntegration that has a skeleton project that can be deployed to IIS to demonstrate the issue. It seemed like too much code to paste inline since it was multiple files.
  3. I enable stdout to get the following message.
Hosting environment: Production
Content root path: C:\Users\*****\source\TestIisIntegration\bin\Release\netcoreapp2.2\win-x64\publish
Now listening on: http://127.0.0.1:5781
Application started. Press Ctrl+C to shut down.
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/1.1 GET http://localhost/  
info: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[1]
      Route matched with {action = "Index", controller = "Home"}. Executing action TestIisIntegration.Controllers.HomeController.Index (TestIisIntegration)
info: Microsoft.AspNetCore.Authorization.DefaultAuthorizationService[2]
      Authorization failed.
info: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[3]
      Authorization failed for the request at filter 'Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter'.
info: Microsoft.AspNetCore.Mvc.ChallengeResult[1]
      Executing ChallengeResult with authentication schemes ().
info: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[2]
      Executed action TestIisIntegration.Controllers.HomeController.Index (TestIisIntegration) in 43.0389ms
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 401.6257ms 401 
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/1.1 GET http://localhost/  
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "*************", Request id "*************:00000002": An unhandled exception was thrown by the application.
System.InvalidOperationException: Session has not been configured for this application or request.
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Session()
   at TestIisIntegration.ClaimsTransformation.TransformAsync(ClaimsPrincipal principal) in C:\Users\erhma5\source\TestIisIntegration\ClaimsTransformation.cs:line 25
   at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 35.5237ms 500 

Expected behavior

Expected behavior is that HttpContext.Session should return null if Session is not available. Or at least there should be a HttpContext.SessionConfigured bool that can safely check. This ClaimsTransformation will be hit constantly and an exception being thrown, caught, and ignored is unnecessary overhead, not to mention messy.

.Net Info

.NET Core SDK (reflecting any global.json):
 Version:   2.2.101
 Commit:    236713b0b7

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

Host (useful for support):
  Version: 2.2.2
  Commit:  a4fd7b2c84

.NET Core SDKs installed:
  1.0.4 [C:\Program Files\dotnet\sdk]
  2.0.2 [C:\Program Files\dotnet\sdk]
  2.0.3 [C:\Program Files\dotnet\sdk]
  2.1.2 [C:\Program Files\dotnet\sdk]
  2.1.4 [C:\Program Files\dotnet\sdk]
  2.1.100 [C:\Program Files\dotnet\sdk]
  2.1.101 [C:\Program Files\dotnet\sdk]
  2.1.102 [C:\Program Files\dotnet\sdk]
  2.1.103 [C:\Program Files\dotnet\sdk]
  2.1.104 [C:\Program Files\dotnet\sdk]
  2.1.200 [C:\Program Files\dotnet\sdk]
  2.1.201 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.302 [C:\Program Files\dotnet\sdk]
  2.1.403 [C:\Program Files\dotnet\sdk]
  2.1.500 [C:\Program Files\dotnet\sdk]
  2.1.502 [C:\Program Files\dotnet\sdk]
  2.1.503 [C:\Program Files\dotnet\sdk]
  2.2.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

Possible Fixes

  1. This line could be changed to the below code, but I would assume this would be breaking change and may have negative downstream impact.
var feature = SessionFeatureOrNull;
if (feature == null)
{
    return null;
}
return feature.Session;
  1. Add SessionConfigured property to DefaultHttpContext. With this, I could wrap my HttpContext.Session calls in any Auth Handler inside of a if (HttpContext.SessionConfigured) {} block.
public bool SessionConfigured
{
    get
    {
        return SessionFeatureOrNull != null;
    }
}
  1. Add GetSession() method to DefaultHttpContext.
public ISession GetSession()
{
    get
    {
        if (SessionFeatureOrNull == null)
        {
            return null;
        }
        return Session;
    }
}

Let me know if any of these solutions sound good and I'll create a pull request.

@fractionalJoe
Copy link
Author

Any thoughts on this?

@mpalmer78
Copy link

@fractionalJoe did you find a work-around to this issue? it is also affecting my project.

@fractionalJoe
Copy link
Author

I did. I created a wrapper service that takes the IHttpContextAccessor as a ctor parameter. It has a HasValidSession() method. I don't have the code in front of me, but as I recall it's:

private bool HasValidSession()
{
    try
    {
        return _httpContext.Session.Id != null;
    }
    catch (Exception)
    {
        return false;
    }
}

Note that _httpContext is set in the ctor from the IHttpContextAccessor.HttpContext. I avoid using try/catch as control logic whenever possible, but this is the only way I've found to do it in the current design.

Once you have that you can use something like"

public void DoSomething()
{
    if (!_sessionService.HasValidSession())
    {
        //...in my testing this only seems to happen with preflight requests, so I fail silently. Your experience may differ so test thoroughly.
        return;
    }
    //.... whatever you really want to do here.
}

@mpalmer78
Copy link

Awesome, I'll give it a shot. Thanks for sharing!

@Tratcher
Copy link
Member

Note this exception is intentional to highlight middleware ordering problems. We should address the ordering issue, not the exception.

That said, this particular ordering issue is difficult. The IIS Middleware is registered ahead of any app middleware like Session, and there's not a good way to insert something ahead of it. We need to re-think this code:
https://github.com/aspnet/AspNetCore/blob/402394347d5a9b3add7decf52af85cbb4e7866b3/src/Servers/IIS/IISIntegration/src/IISMiddleware.cs#L131-L139
https://github.com/aspnet/AspNetCore/blob/402394347d5a9b3add7decf52af85cbb4e7866b3/src/Servers/IIS/IISIntegration/src/AuthenticationHandler.cs#L24-L35
AuthenticateAsync is called eagerly in order to initialize the WindowsIdentity and make sure all of the handles are taken care of. We could do that initialization directly without calling AuthenticateAsync. AuthenticateAsync would still be called alter by the AuthenticationMiddleware, Authorization, etc..

@Tratcher Tratcher added area-servers bug This issue describes a behavior which is not expected - a bug. feature-iis Includes: IIS, ANCM breaking-change This issue / pr will introduce a breaking change, when resolved / merged. and removed area-middleware labels Jun 12, 2019
@Tratcher Tratcher changed the title Session has not been configured for this application or request IIS (out-of-proc) calls AuthenticateAsync for all requests at the start of the pipeline Jun 12, 2019
@fractionalJoe
Copy link
Author

That makes sense to me conceptually. Looking at the log, AuthenticateAsync invokes ClaimsTransformation.TransformAsync but I don't see where that occurs. Can you point me in the right direction?

@Tratcher
Copy link
Member

Tratcher commented Jul 1, 2019

Verified using SDK 3.0.100-preview7-012742

@Tratcher Tratcher added the accepted This issue has completed "acceptance" testing (including accessibility) label Jul 1, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions breaking-change This issue / pr will introduce a breaking change, when resolved / merged. bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed feature-iis Includes: IIS, ANCM
Projects
None yet
Development

No branches or pull requests

6 participants