dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.15k stars 9.92k forks source link

OpenIdConnectAuthenticationHandler: message.State is null or empty #55910

Open merijndejonge opened 3 months ago

merijndejonge commented 3 months ago

Is there an existing issue for this?

Describe the bug

I'm using Microsoft Identity and and Microsoft Entra as external identity server. I'm using AddOpenIdConnect to set it all up. Everything works perfectly fine except when Microsoft is making a callback as a result of granting tenant-wide admin consent for my app. This callback is not handled correctly and issues the following Exception: [2024-05-25` 22:22:08 ERR] An unhandled exception has occurred while executing the request. Microsoft.AspNetCore.Authentication.AuthenticationFailureException: An error was encountered while handling the remote login.

 ---> Microsoft.AspNetCore.Authentication.AuthenticationFailureException: OpenIdConnectAuthenticationHandler: message.State is null or empty.
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync()
   at Duende.IdentityServer.Hosting.FederatedSignOut.AuthenticationRequestHandlerWrapper.HandleRequestAsync() in /_/src/IdentityServer/Hosting/FederatedSignOut/AuthenticationRequestHandlerWrapper.cs:line 38
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
   at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Tenant-wide admin consent is granted using the URL https://login.microsoftonline.com/{organization}/adminconsent?client_id={client-id} as documented at https://learn.microsoft.com/en-us/entra/identity/enterprise-apps/grant-admin-consent?pivots=portal#construct-the-url-for-granting-tenant-wide-admin-consent.

It triggers the following callback to my identity server:

https://<my-server>/signin-microsoft?admin_consent=True&tenant=<teant-id>

The path /signin-microsoft is correct and according to my setup.

As a result the sysadmin who want to give tenant-wide consent to my app will get an error from my backend as a result. Obviously, this is not desired.

Expected Behavior

The expected behaviour is that the sys admin will get as normal HTTP 200 response to indicate success. Or else, there should be an option that my backend can handle the request and returns a HTML result page or something.

Steps To Reproduce

Open the URL https://<my-server>/signin-microsoft?admin_consent=True&tenant=<teant-id> in a browser to trigger the exception.

Exceptions (if any)

---> Microsoft.AspNetCore.Authentication.AuthenticationFailureException: OpenIdConnectAuthenticationHandler: message.State is null or empty. --- End of inner exception stack trace --- at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync() at Duende.IdentityServer.Hosting.FederatedSignOut.AuthenticationRequestHandlerWrapper.HandleRequestAsync() in /_/src/IdentityServer/Hosting/FederatedSignOut/AuthenticationRequestHandlerWrapper.cs:line 38 at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context) at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext) at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

.NET Version

net8.0

Anything else?

No response

merijndejonge commented 3 months ago

You can use the blazor sample

Which is recommended by MS (https://learn.microsoft.com/en-us/aspnet/core/blazor/security/blazor-web-app-with-oidc?view=aspnetcore-8.0&pivots=without-bff-pattern)

This will trigger the following log:

fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      Microsoft.AspNetCore.Authentication.AuthenticationFailureException: An error was encountered while handling the remote login.
       ---> Microsoft.AspNetCore.Authentication.AuthenticationFailureException: OpenIdConnectAuthenticationHandler: message.State is null or empty.
         --- End of inner exception stack trace ---
         at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync()
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)
halter73 commented 3 months ago

I'll see what we can do to improve OpenIdConnectAuthenticationHandler and/or our Blazor documentation to avoid this issue with tenant-wide admin consent.

It's a little surprising that it's reusing the signin redirect URI (e.g. /signin-microsoft or /signin-oidc) URL when redirecting after admin consent rather than a URL that's unique from signin designed to handle admin consent completing for that application. I wonder if it's because you aren't specifying a redirect_uri, and the signin redirect URI is the only one registered in the app registration portal, so Entra just uses that.

Looking at the documentation from https://learn.microsoft.com/en-us/entra/identity-platform/v2-admin-consent#request-the-permissions-from-a-directory-admin, it seems like they show it redirecting back to a unique /permissions endpoint which is specified via a&redirect_uri query param in the original https://login.microsoftonline.com/{tenant}/v2.0/adminconsent?client_id=... URL. The docs claim you can use any redirect_uri registered in the app registration portal, so your best bet might just be able to fix your issue by registering a new /permissions URI in the portal, specifying that as your redirect_uri, and writing a custom endpoint to handle the /permissions request.

Another thing to consider is to try using AddMicrosoftIdentityWebApp instead of using AddOpenIdConnect and AddCookie directly. It adds a bunch of customizations on top of AddOpenIdConnect and AddCookie that would probably provide better integration to non-standard features provided by Entra, although I'm not sure if it specifically accounts for this.

If you have to use the signin redirect URI to handle this the admin consent redirect, you can set OpenIdConnectOptions.SkipUnrecognizedRequests which should avoid this error. If you want to redirect back to a specific page in this case, you can configure an OpenIdConnectOptions.Events.OnMessageReceived callback that calls messageReceivedContext.SkipHandler() after calling httpContext.Response.Redirect(wherever) itself.

You can find the relevant code in the handler that triggers the OnMessageReceived event and checks SkipUnrecognizedRequests.

https://github.com/dotnet/aspnetcore/blob/0ffc29649a8047ad969445fd185450892b3fc75b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs#L579-L602

merijndejonge commented 3 months ago

Thanks for the clear explanation. I wasn't aware of the redirect_uri because it wasn't mentioned in the example thatI referred to (https://learn.microsoft.com/en-us/entra/identity/enterprise-apps/grant-admin-consent?pivots=portal#construct-the-url-for-granting-tenant-wide-admin-consent)

Since the grant-admin-consent URL is the be used by my clients it is pretty fragile to also request them to use the redirect_uri. My proposal therefore would be to make redirect_uri optional and do nothing in case this parameter is absent in the grant-admin-consent request. This, in my view, is the most robust and best way.

To make the current approach robust and customer-friendly I must make available an open endpoint just for the redirect_url. This is not only inconvenient and undesired, opening an unauthenticated endpoint is also not optimal from a security point of view because anyone can use this unnecessary endpoint.

I've also tried the SkipUnrecognizedRequests approach. This, indeed, prevents the exception but it gives a general 404 error, which is also not a customer-friendly solution.

In order to combine it with the OnMessageReceived suggestion, I need a bit more help. Do you suggest to check in the OnMessageReceived handler wether the request URL contains something like "signin-oidc?admin_consent=True" and then change this URL to public accessible page, like /permissions as in the example?

merijndejonge commented 2 months ago

any news/updates on this?