aspnet / AspNetKatana

Microsoft's OWIN implementation, the Katana project
Apache License 2.0
967 stars 334 forks source link

Unable to have multiple OpenID Connect middlewares #188

Closed RobSiklos closed 4 years ago

RobSiklos commented 6 years ago

I've configured multiple OpenID Connect middlewares, each having a different value for OpenIdConnectAuthenticationOptions.AuthenticationType, and registered with: app.UseOpenIdConnectAuthentication(options).

The problem seems to be that when the authentication occurs, the first registered middleware always handles things, instead of the middleware corresponding to the correct authentication type.

Lets say my two middlewares have AuthenticationTypes of "FirstProviderAuthType" and "SecondProviderAuthType".

I'm kicking off the authentication using something like this:

var properties = new AuthenticationProperties { RedirectUri = "https://something", };
((IOwinContext)context).Authentication.Challenge(properties, "SecondProviderAuthType");

However, in any of the notifications (e.g. OpenIdConnectAuthenticationOptions.Notifications.SecurityTokenReceived), the value of notification.Options.AuthenticationType is always equal to "FirstProviderAuthType", which is definitely not what I would expect.

Using all 4.0.0 Katana libraries from NuGet.

Tratcher commented 6 years ago

Each instance must specify a unique CallbackPath to avoid this ambiguity.

RobSiklos commented 6 years ago

Thanks @Tratcher. I tried specifying this, but now it seems like the RedirectUri specified in the AuthenticationProperties is no longer honoured. Any idea what this means?

Tratcher commented 6 years ago

No, that's pretty vague. Are your events at least working as expected?

RobSiklos commented 6 years ago

I'll try and get you a test project which demonstrates the issue.

RobSiklos commented 6 years ago

Ok - here's a minimal sample project showing a couple bits of weirdness: OpenIdConnectTest.zip

First, you'll need to customize your web app URL and OIDC identity provider information in Startup.cs (lines 65-76, plus 89).

When you run the app, browse to a URL like this: "http://localhost:12666/Home/Auth?authenticationType=AAD-OpenIdConnect", which will authenticate according to the specified auth type.

At first, everything should work fine. Once you've authenticated with the IdP, it should kick you back to /Home/ExternalAuthCallback.

Now, try uncommenting line 51 in Startup.cs. This puts in a custom data protection provider. The important thing here is that the data protection provider does not consider the "purposes" at all. When you run the app again, and try to authenticate again using the second OIDC middleware, it should fail, because it's trying to handle it using the first middleware.

It looks like I can solve this by properly implementing the custom data protection provider to consider the purposes, and throw a CryptographicException if the purposes don't match, but I still think it should work without this change.

Although I think it should be enough that my two middlewares have different values for AuthenticationType, I tried your suggestion of specifying a unique value for CallbackPath (uncomment line 92 in Startup.cs). However, once I make this change, I no longer get redirected to /Home/ExternalAuthCallback like I used to before. This is probably a separate issue from the data protection provider thing, so let me know if you want me to enter another ticket.

Tratcher commented 6 years ago

You're setting RedirectUri, not CallbackPath, and it must be unique per instance.

RobSiklos commented 6 years ago

@Tratcher You're talking about setting CallbackPath in the OpenIdConnectAuthenticationOptions right? Do I need to set RedirectUri as well, or is CallbackPath enough?

Using the sample application I provided, I set a unique callback path per identity provider (by uncommenting line 92 in Startup.cs). However, once I make this change, I no longer get redirected /Home/ExternalAuthCallback, which is what I specified as the value of AuthenticationProperties.RedirectUri which I provide when making the challenge (HomeController.cs line 27).

Tratcher commented 6 years ago

RedirectUri isn't required. What did you set CallbackPath to?

RobSiklos commented 6 years ago

I tried setting CallbackPath to all sorts of things. Even with only a single OIDC middleware, the browser ends up at "/" after authentication is complete.

Tratcher commented 6 years ago

Don't set RedirectUri, and set CallbackPath to /signin-oidc1 and /signin-oidc2 on the respective instances. These need to be unique paths so the auth handler can filter out their own requests. You won't have MVC endpoints for these.

RobSiklos commented 6 years ago

Yes, that's what I'm doing. I am setting CallbackPath on the options to a unique name, and just to make sure, I've only registered a single instance of the OIDC middleware.

I also commented out the code which sets RedirectUri both on the OpenIdConnectAuthenticationOptions as well as the authentication properties for the Challenge.

No matter what I do, I end up at "/" in the browser after authentication is complete.

Did you try my sample application? It demonstrates the issue.

MoonStorm commented 6 years ago

It seems that the logic wasn't fully implemented. This is the only usage of the callback path.

if (this.Options.CallbackPath.HasValue && this.Options.CallbackPath != this.Request.PathBase + this.Request.Path)
        return (AuthenticationTicket) null;

but nothing to tell the identity provider to come back with its response on this path.

The workaround I found was to hook up to the RedirectToIdentityProvider callback and set the ProtocolMessage.RedirectUri property with the full URL of the callback path. The CallbackPath is still required to be set with the URL relative to the website (thus including the app path) for the handler to kick in.

new OpenIdConnectAuthenticationOptions(){
 ...
 Notifications = new OpenIdConnectAuthenticationNotifications()
                        {
                            RedirectToIdentityProvider = notification =>
                                {
                                    var authenticationResponseChallenge = notification.OwinContext.Authentication?.AuthenticationResponseChallenge;

                                    if (authenticationResponseChallenge != null)
                                    {
                                        // we're here which means we're logging in
                                        notification.ProtocolMessage.RedirectUri = callbackAbsoluteUri;
                                    }
                         }
}

I wouldn't recommend using the Microsoft OpenIdConnect module in production at this moment. It doesn't seem like it's fully implemented and it's definitely not well tested.

Tratcher commented 6 years ago

@MoonStorm this component has been released and running in production scenarios for four years now.

The CallbackPath in this case is derived from Options.RedirectUri: https://github.com/aspnet/AspNetKatana/blob/e2b18ec84ceab7ffa29d80d89429c9988ab40144/src/Microsoft.Owin.Security.OpenIdConnect/OpenidConnectAuthenticationHandler.cs#L145 https://github.com/aspnet/AspNetKatana/blob/e2b18ec84ceab7ffa29d80d89429c9988ab40144/src/Microsoft.Owin.Security.OpenIdConnect/OpenIdConnectAuthenticationMiddleware.cs#L50-L58

Setting both RedirectUri and CallbackPath to mismatched values may cause some of the above confusion.

MoonStorm commented 6 years ago

@Tratcher Not sure what you're trying to show me. How the CallbackPath is inferred from other options?

Could you please give us a small sample as to what we need to set to have the CallbackPath actually do what's supposed to do?

Tratcher commented 6 years ago

I'd forgotten that OpenIdConnectAuthenticationOptions.RedirectUri is the primary value in this version of OIDC, we've removed it from the ASP.NET Core version. Setting RedirectUri to an absolute uri like "http://localhost/custom-signin-oidc" should be adequate for most scenarios, there's no need to set CallbackPath as it will be derived from RedirectUri.

CallbackPath primarily needs to be overridden when your app is hosted as a sub site like "http://localhost/mysite/". In that case CallbackPath needs to be set to "/signin-oidc" and RedirectUri needs to be set to "http://localhost/mysite/signin-oidc". "/mysite" is trimmed by the server before the request reaches the OIDC middleware.

praveena-muk commented 5 years ago

@MoonStorm / @RobSiklos based on the discussion above i understand this issue is the scenario when OWIN startup + multiple OpenIdConnect Authorities + custom DataProtector are involved. were you able to solve your issue? (asking since the thread is still open) could you maybe post a sample solution?