DuendeSoftware / IdentityServer

The most flexible and standards-compliant OpenID Connect and OAuth 2.x framework for ASP.NET Core
https://duendesoftware.com/products/identityserver
Other
1.45k stars 337 forks source link

Identity Server Login and Logout Localization Limitations #1557

Open PascalAlbrechtComparis opened 4 months ago

PascalAlbrechtComparis commented 4 months ago

Which version of Duende IdentityServer are you using? 6.3.8

Which version of .NET are you using? 6.0.29

Describe the issue

We are using the default ASP.NET Core request localization with resource files. While implementing the ui_locales for login and logout, we encountered two difficulties:

  1. As far as I understood, you are supposed to use the IIdentityServerInteractionService to get the authorization or logout context in the according handler methods (as done in the IdentityServer.Templates). From there you can obtain the validated ui_locales. However, at this point the ASP.NET Core request localization has already determined the culture and initialized the IHtmlLocalizer/IViewLocalizer accordingly. Therefore, it is not possible to switch the localization at this point anymore, to get the desired localized strings.
  2. Not all error cases are providing ui_locales in the context, even when present in the request. This prevents localization of certain errors.

Expected behaviour

I checked the Identity Server code and saw that this behaviour is implemented in the request validators and endpoints. Most of them are internal and not easily modifiable in that regard. And handling the complete endpoint implementation is far more than we like to handle.

We would need a consistent and efficient way to obtain the validated ui_locales in a RequestCultureProvider implementation. As far as I understand, the current implementation is not designed for that and would require changes. Would you consider such changes to the ui_locales handling?

Additional context

I created a workaround by extending the EndSessionRequestValidator and wrapping the AuthorizeRequestValidator (decorating the default Identity Server dependency injection) to hook in some logic for updating our session from the raw ui_locales (in every case). The validator interfaces are public and therefore seem like legit extension points. However, (ab)using the validators in this manner seems unclean.

AndersAbel commented 4 months ago

This is valid point, the current IdentityServer handling of the ui_locales does not work well with the Asp.Net Core localization engine.

I have another suggestion on how to implement this that I think would be much more simple.

The ui_locales parameter is sent in request to protocol endpoints (authorize or endsession). We never return any html from directly from those endpoints; we always redirect to a page (Razor Page, MVC action or anything configured). The Asp.Net Core localization system has a built in cookie for culture selection. Whenever we encounter a ui_locales parameter at one of the protocol endpoints we could set the culture selection cookie accordingly. When the user is redirected to the login or logout page the cookie would be picked up by the built in Asp.Net Core localization handling.

Setting the cookie would be a global opt in on IdentityServer options. Do you think that would work?

josephdecock commented 4 months ago

I like this idea, but I'd like to turn it on by default. Is there a risk of breakage if you're not expecting it or something?

AndersAbel commented 4 months ago

If you already have set the cookie yourself you might be surprised if IdentityServer overwrites it. But I guess it would be a rare case that you wouldn't want to honor the ui_locales. It would probably be fine to have it enabled by default as long as there is an easy way to disable it.

PascalAlbrechtComparis commented 4 months ago

If I understand you correctly, you intend to directly set the default .AspNetCore.Culture cookie. I see two points to consider:

  1. The solution might work well if the CookieRequestCultureProvider is used. However, the cookie may require special handling if other providers (query parameter, route, session, etc.) are used to maintain the culture. We are storing the selected culture in the session and do not want the cookie to override later user choices. However, we should be able implement some logic to update the state and remove the cookie afterwards. This might be slightly awkward, though.
  2. In our case, the ui_locales contains multiple locales (as specified in OpenID Connect Core). However, the ASP.NET Core culture cookie is designed to contain only one request culture. Are you planning to implement an algorithm that select the best match based on the supported cultures? Would your implementation be adjustable if it does not fit our requirements?

I thought that it might be possible to provide the validated ui_locales through a URL parameter on the redirects of the protocol endpoints. However, I see that this likely adds more complexity on the redirects of the protocol endpoints compared to setting a cookie.

AndersAbel commented 4 months ago

Thanks for your feedback @PascalAlbrechtComparis!

We should probably surface the ui_locales in an extension point. The default implementation can be to set the .AspNetCore.Culture cookie, but it would allow you to handle it yourself. If there are multiple values I think that the most reasonable thing the default implementation is to pick the first ui_locales value that is registered as supported.

We did discuss if it would be possible to keep the ui_locales available through all redirects and if it would be possible to handle them early in the pipeline to integrate better with Asp.Net Core. Doing that turns out to be a large change and it requires careful consideration of how our validation system is invoked. It would be much easier to use the ui_locales as captured on the original endpoint request and then store the preference in a cookie (or other session based storage) when redirecting to the UI pages.

if we surface the ui_locales on an extension point, would that make it possible for you to implement a solution with your existing logic and session based storage?

PascalAlbrechtComparis commented 4 months ago

Thanks for your considerations, @AndersAbel.

Yes, if we could hook in our own implementation to update the request culture, that should work quite well. As far as I understand, we would be able to customise the processing of the ui_locales into the request culture and how we store it. That's generally what we implemented in our workaround within the validators.

It would probably be convenient if these ui_locales are validated. We had to duplicate the default validation in our workaround.

Regarding the selection of the matching request culture: We implemented some logic to provide the best matching language, even if there is no exact culture match. This seems to be the recommend behaviour. However, if we can provide our own implementation, I see no issue if the default implementation is less sophisticated.

AndersAbel commented 4 months ago

Thanks @PascalAlbrechtComparis for your input. It looks like we have an idea of a solution that both would work out of the box for simple cases but also be extendable for someone with more advanced requirements.