Middleware does not pass OwinContext.User forward in Pipeline

[daniefer]

I am trying to setup a web api with signalr and during the process I noticed this oddity. I cannot seem to find a documented reason why the Identity on the OwinContext would be emptied out after there was no match on the web api route table. I put together a simple project to show this in action:

Startup

public void Configuration(IAppBuilder appBuilder)
{
    var config = new HttpConfiguration();
    config.Filters.Clear();
    config.SuppressDefaultHostAuthentication();
    appBuilder.UseOAuthBearerAuthentication(new OAuthBearerAuthenticationOptions
    {
        AuthenticationMode = AuthenticationMode.Active,
        AccessTokenFormat = new TokenFormatter(),
        AuthenticationType = "DummyAuth",
        Provider = new OAuthBearerAuthenticationProvider
        {
            OnValidateIdentity = async (ctx) => ctx.Validated(ctx.Ticket),
            OnRequestToken = async (ctx) => ctx.Token = "111"
        }
    });
    appBuilder.Use<DebugMiddleware>("New Request", (Action<IOwinContext>)((IOwinContext ctx) => Console.WriteLine("End Request")));
    appBuilder.UseCors(CorsOptions.AllowAll);
    appBuilder.Use<DebugMiddleware>("Before WebApi");
    config.MapHttpAttributeRoutes();
    appBuilder.UseWebApi(config);
    appBuilder.Use<DebugMiddleware>("After WebApi");
    appBuilder.RunSignalR();
    appBuilder.Use<DebugMiddleware>("After SignalR");
    config.EnsureInitialized();
}

Dummy token formatter

public class TokenFormatter : ISecureDataFormat<AuthenticationTicket>
{
    public string Protect(AuthenticationTicket data)
    {
        return "111";
    }
    public AuthenticationTicket Unprotect(string protectedText)
    {
        var identity = new ClaimsIdentity("DummyAuth");
        identity.AddClaim(new Claim(ClaimTypes.NameIdentifier, "Bob"));
        identity.AddClaim(new Claim(ClaimTypes.Name, "Bob"));
        var ticket = new AuthenticationTicket(identity, null);
        return ticket;
    }
}

Controller

[Authorize]
public class TestController : ApiController
{
    [Route("Value")]
    [HttpGet]
    public string GetValue()
    {
        return "Hello World!";
    }
}

Hub

public class TestHub : Hub
{
    public override Task OnConnected()
    {
        Console.WriteLine($"Hub.OnConnected Username: {new OwinContext(Context.Request.Environment).Authentication?.User?.Identity?.Name}");
        return base.OnConnected();
    }
    public override Task OnDisconnected(bool stopCalled)
    {
        Console.WriteLine($"Hub.OnDisconnected Username: {new OwinContext(Context.Request.Environment).Authentication?.User?.Identity?.Name}");
        return base.OnDisconnected(stopCalled);
    }
}

If I attempt to connect to a Hub (at the end of the pipeline) the OwinContext is no longer authenticated.
The output from each DebugMiddleware shows:

Output

Start Request
Authenticated?: True
User: Bob
Before WebApi
Authenticated?: True
User: Bob
After WebApi
Authenticated?: False <- Why the change here?
User:
End Request

Is this a bug or is there a reason for this maddening quirk?

[mkArtakMSFT]

Hi @daniefer. Thanks for all the details you've provided. Do you happen to have a small sample project we can use to repro the issue? //cc @danroth27, @rynowak

[daniefer]

Cleaned up the example a bit and made a repo here https://github.com/daniefer/AspWebApiIssueExample. Thanks @mkArtakMSFT for getting this started.

[daniefer]

Any update or ideas @mkArtakMSFT / @danroth27 / @rynowak ?

[mkArtakMSFT]

@dougbu, do you have any thoughts about this?

[dougbu]

Clearing the IOwinContext.User property is mainly the result of calling HttpConfiguration.SuppressDefaultHostAuthentication() in the Startup class. That method adds the PassiveAuthenticationMessageHandler which (in part) sets OwinHttpRequestContext.Principal to the anonymous user. That Principal property writes through to IOwinContext.User.

Workaround

The test application seems to work as expected if I remove the SuppressDefaultHostAuthentication() call.

If you need that call, I suggest creating a new message handler (DelegatingHandler subclass) to wrap PassiveAuthenticationMessageHandler (i.e. use PassiveAuthenticationMessageHandler as the InnerHandler) and inserting that handler instead of calling SuppressDefaultHostAuthentication(). The handler should save IOwinContext.User and restore it after the usual base.SendAsync(...) call.

Potential fix

Changing PassiveAuthenticationMessageHandler to restore HttpRequestContext.Principal after its base.SendAsync(...) call seems like the obvious fix. But, the code may be as it is for a reason i.e. not just because we were thinking Web API would always be at the end of the Owin pipeline. That is, we may need to add a configuration switch to opt into (or out of) the new behaviour to avoid a breaking change.

[dougbu]

Don't need much more investigation. But, I'm leaving the investigate label 'til we discuss whether my suggested fix is a breaking change.

BTW we should change SuppressHostPrincipalMessageHandler if we update PassiveAuthenticationMessageHandler. That also sets but does not restore HttpRequestContext.Principal.

[abatishchev]

Adding public bool RestorePrincipal { get; set; } which naturally will be by default false sounds to me as the right approach.

[dougbu]

We discussed this internally and decided two things:

1. Many, likely most, Web API scenarios will be totally unaffected if we change PassiveAuthenticationMessageHandler and SuppressHostPrincipalMessageHandler to restore the Principal before SendAsync(...) returns. Must both use an Owin host and perform authentication within the Owin pipeline (not the inner Web API pipeline) to see this issue.
   Admittedly, could see something similar with another custom host that has an independent "outer" pipeline of its own.
2. It's Just Wrong:tm: not to restore the Principal.

Therefore, we should fix this problem without a fallback to the previous, incorrect behaviour.

[dougbu]

Clearing assignment and investigate label. Over to the triage team…

[daniefer]

Thanks for investigation everyone. It is good to hear that I am not losing my mind.

Another bit of info that might be useful, I was able to work around this without adding a wrapper around the PassiveAuthenticationMessageHandler. The issue does not appear if I put the web api on a sub route, e.g.:

   appBuilder.Map("api" app => {
      app.UseWebApi(config);
   };
   appBuilder.RunSignalR();

Luckily we were already prefixing all our routes with a consistent name so it was a pretty simple transition.

[mkArtakMSFT]

Thanks @dougbu , @daniefer. I'm closing this issue as it seems the workaround was pretty cheap to apply.

[mkArtakMSFT]

Reopening, as the actual fix is very cheap too... Better to do it and forget!

[dougbu]

f9e06d6