Skip to content

Allow acr_values and ui_locales to be specified in OpenIdConnectOptions #39503

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

Open
ghost opened this issue Jan 13, 2022 · 6 comments
Open

Allow acr_values and ui_locales to be specified in OpenIdConnectOptions #39503

ghost opened this issue Jan 13, 2022 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer

Comments

@ghost
Copy link

ghost commented Jan 13, 2022

Background and Motivation

Currently to set the acr_values and ui_locales parameters in the authorization request (https://openid.net/specs/openid-connect-core-1_0.html section 3.1.2.1. Authentication Request), we need to use the OnRedirectToIdentityProvider event like:

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddAuthentication().AddOpenIdConnect(options =>
{
   options.Events.OnRedirectToIdentityProvider = context =>
   {
       context.ProtocolMessage.AcrValues = "tenant:abc";
       context.ProtocolMessage.UiLocales = "en-us"
       return Task.CompletedTask;
   };
});

It would be interesting to add these two properties directly in OpenIdConnectOptions.

The mapping should be easy (here https://github.com/dotnet/aspnetcore/blob/main/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs#L382) as these two properties already exist in the OpenIdConnectMessage (https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectMessage.cs)

Proposed API

namespace Microsoft.AspNetCore.Authentication.OpenIdConnect;

public class OpenIdConnectOptions : RemoteAuthenticationOptions
{
+    public string? AcrValues { get; set; }
+    public string? UiLocales { get; set; }
}

Usage Examples

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddAuthentication().AddOpenIdConnect(options =>
{
    options.AcrValues = "tenant:abc";
    options.UiLocales = "en-us";
});

Alternative Designs

We can maybe rely on the new AdditionalAuthorizationParameters proposed in #39243 to set these two parameters but should we reserve this property only for non standard OAuth/OpenID parameters?

Risks

Nothing I can think of now.

cc @Tratcher @martincostello

@ghost ghost added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 13, 2022
@mkArtakMSFT mkArtakMSFT added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jan 13, 2022
@martincostello
Copy link
Member

martincostello commented Jan 14, 2022

We have something similar to AdditionalAuthorizationParameters in the AspNet.Security.OpenId.Providers package:

https://github.com/aspnet-contrib/AspNet.Security.OpenId.Providers/blob/1a826454dea02d73712a550532b14b4c9cc07180/src/AspNet.Security.OpenId/OpenIdAuthenticationHandler.cs#L328-L337

Something similar to that is probably best, rather than trying to introduce strongly-typed parameters for ACR and UI, as then it can be a bit of a slippery slope of "oh, can we add Foo too?".

Ultimately those properties are just backed by a dictionary anyway (AuthenticationProtocolMessage.Parameters) when they're accessed via SetParameter(string, string).

A more generic solution would be something like AdditionalAuthorizationParameters that could loop through an IDictionary<string, string> and add them to the message before the redirect. You'd just need to know the property names for configuring them (e.g. OpenIdConnectParameterNames.AcrValues) on OpenIdConnectOptions.

@adityamandaleeka adityamandaleeka added this to the .NET 7 Planning milestone Jan 14, 2022
@ghost
Copy link

ghost commented Jan 14, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@adityamandaleeka
Copy link
Member

We can maybe rely on the new AdditionalAuthorizationParameters proposed in #39243 to set these two parameters

This is the approach we should probably go with, so this is consistent with how we specify the other properties.

@Ponant
Copy link
Contributor

Ponant commented Jan 17, 2022

Hello,
I faced a similar issue, and found out that also redirection back should/could be addressed, when users cancel the oidc flow. See AzureAD/microsoft-identity-web#1596

@ghost
Copy link

ghost commented Sep 13, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Eagle3386
Copy link

@mkArtakMSFT & @javiercn
Since the workaround suggested in #45832 can't be done anymore (there's no options.Events anymore), how am I supposed to add ui_locales to my MSAL setup in my Blazor WASM standalone app that connects to our Azure B2C tenant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Projects
None yet
Development

No branches or pull requests

6 participants