Skip to content
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

There is no way to set the DisplayName for the OpenIdConnect provider #808

Closed
Shazwazza opened this issue Dec 4, 2020 · 16 comments
Closed
Labels
Milestone

Comments

@Shazwazza
Copy link
Contributor

Which version of Microsoft Identity Web are you using?
1.3.0

Where is the issue?

  • Other (please describe)

It's not possible to set the display name since the call to AddOpenIdConnect is all done internally here

which doesn't pass in a display name and a display name is not configurable by AuthenticationSchemeOptions

Is this a new or an existing app?
c. This is a new app or an experiment.

Expected behavior
There should be a parameter in AddMicrosoftIdentityWebApp to supply a DisplayName

Actual behavior
There is not an option to do this

@Shazwazza
Copy link
Contributor Author

I'm happy to send a PR to add this

@jmprieur
Copy link
Collaborator

jmprieur commented Dec 4, 2020

The display name (of the signed-in user, I'm assuming) is inferred from the claims which are in the IDToken. There is no need to specify it externally. What is your scenario? what are you trying to do?. In which case would you want to override a display name which is not in the claims?

To answer your question:

  1. you have full control of your UI anyway. When you unfold one of the ASP.NET Core templates, the page displaying the name is Pages/Shared/_LoginPartial, which is in the code of your app (not in the framework / the library).

  2. if you don't want to change the UI, the display comes from the options.TokenValidationParameters.NameClaimType

    if (microsoftIdentityOptions.Value.IsB2C)
    {
    options.TokenValidationParameters.NameClaimType = ClaimConstants.Name;
    }
    else
    {
    options.TokenValidationParameters.NameClaimType = ClaimConstants.PreferredUserName;
    }

    You can always override after calling AddMicrosoftIdentityWebApp

    services.Configure<OpenIdConnectOptions>(OpenIdConnectDefaults.AuthenticationScheme, options =>
    {
        options.TokenValidationParameters.NameClaimType = nameOfTheClaimYouWantToUse;
    });

@jmprieur jmprieur added answered question Further information is requested labels Dec 4, 2020
@Shazwazza
Copy link
Contributor Author

Hi there, what I am referring to by DisplayName is the display name of the provider.

The code is what I mentioned above with this call builder.AddOpenIdConnect. This method has an optional displayName parameter which must be set via this call as its not settable by any options configuration. This is the overload I'm referring to which sets "A display name for the authentication handler."

@jmprieur jmprieur added enhancement New feature or request and removed answered more info needed question Further information is requested labels Dec 7, 2020
@jmprieur
Copy link
Collaborator

jmprieur commented Dec 7, 2020

Thanks for the update, @Shazwazza
This makes sense.

@farshid3003
Copy link

I have this problem as well, we cannot set the provider display name

@jmprieur jmprieur added this to the 1.4.1 milestone Dec 8, 2020
@jmprieur
Copy link
Collaborator

jmprieur commented Dec 8, 2020

@Shazwazza @farshid3003
would it work if we predefined a better name like "Microsoft identity platform" ? or "Azure active directory" ? or do you need to provide your own label? (what would it be?)

@Shazwazza
Copy link
Contributor Author

@jmprieur Since this lib can be used as boilerplate configuration for Azure B2C then the label could really be anything since B2C is a white label auth system that can be branded in any way.

@jmprieur jmprieur modified the milestones: 1.4.1, 1.6.0 Jan 21, 2021
@theyo
Copy link

theyo commented Jan 28, 2021

@jmprieur Just ran into this issue today. Having a default predefined name like "Microsoft Identity Platform" would be great to keep it unique compared to the basic OIDC provider. However, it is also beneficial to be able to change the name in cases where we might need to register mutliple instances of the Microsoft Identity provider as in the case of using something like Identity Server to provide a single federated auth system for a suite of applications.

@jmprieur
Copy link
Collaborator

jmprieur commented Jan 29, 2021

@jennyf19. I propose we take this one as well for next release.

Spec.

  1. In this line, we need to pass-in a new string parameter the identityProviderName (AddOpenIdConnect method has overrides)

  1. To pass-in this parameter, we cannot do it through the MicrosoftIdentityOptions as they are not yet available at that time (they will be on the call back to the creation of the OpenIdConnect scheme), so I believe we need to pass it as an additional optional parameters to all the AddMicrosoftIdentityWebApp methods
string identityProviderDisplayName = "Microsoft identity platform"

@theyo
Copy link

theyo commented Jan 29, 2021

I would also ask that the implementation is consistent with the the built-in providers here: https://github.com/dotnet/aspnetcore/tree/2670c128d522473e146ff9f8159bfffdfe694cd9/src/Security/Authentication

Namely, that the parameter name is displayName and if possible have the parameter order in the extension methods match. The addition of a MicrosoftIdentityWebDefaults class would also be nice.

@jmprieur
Copy link
Collaborator

@theyo : what is the rationale for this ask? the displayName makes sense for AddOpenIdConnect, but AddMicrosoftIdentityWebApp does way more than that.
Would we agree with identityProviderDisplayName ?

@theyo
Copy link

theyo commented Jan 30, 2021

@jmprieur I don't think the comparison is between AddMicrosoftIdentityWebApp and AddOpenIdConnect. It should really be compared to one of the other wrappers like AddMicrosoftAccount. I agree that AddMicrosoftIdentityWebApp does a lot, but at the end of the day it's a wrapper around AddOpenIdConnect just in the same manner that AddMicrosoftAccount is a wrapper around AddOAth.

This method from AddMicrosoftAccount shows how displayName is being passed down into the wrapped AddOAuth method:
https://github.com/dotnet/aspnetcore/blob/2670c128d522473e146ff9f8159bfffdfe694cd9/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountExtensions.cs#L67

Each of the other built-in ASP.NET providers (Google, Twitter, Facebook) expose displayName and pass it into the specific provider they are wrapping in the same manner and this has been the pattern the community typically follows for other provider implementations. Therefore, I believe it makes sense to follow the pattern for the AddMicrosoftIdentityWebApp implemention as well.

@jennyf19 jennyf19 modified the milestones: 1.6.0, 1.6.1 Feb 12, 2021
@apondman
Copy link

apondman commented Feb 15, 2021

there is a workaround you can use right now until the displayName parameter is more easily available:

services.PostConfigure<AuthenticationOptions>(options =>
          {
              foreach (var scheme in options.Schemes)
              {
                  if (scheme.Name == OpenIdConnectDefaults.AuthenticationScheme)
                  {
                      scheme.DisplayName = "My Desired Display Name";
                  }
              }
          });

or register the scheme with a different name:

.AddMicrosoftIdentityWebApp(section, "MyCustomAuthScheme");

and replace the line in the first code snippet above with:

if (scheme.Name == "MyCustomAuthScheme")

@jmprieur jmprieur modified the milestones: 1.10.0, 1.11.0 Apr 22, 2021
@jmprieur jmprieur modified the milestones: 1.11.0, 1.12.0 May 6, 2021
@jmprieur jmprieur modified the milestones: 1.12.0, 1.13.0 Jun 2, 2021
@jmprieur
Copy link
Collaborator

jmprieur commented Jun 2, 2021

@Tratcher FYI we still need to do that.

@jennyf19
Copy link
Collaborator

jennyf19 commented Aug 4, 2021

I have a branch: https://github.com/AzureAD/microsoft-identity-web/tree/jennyf/displayName
with the changes to handle providing the DisplayName, if you want to try it out. Still a work in progress.
@Shazwazza @apondman @theyo @farshid3003

@jennyf19 jennyf19 added this to the 1.16 milestone Aug 5, 2021
@jennyf19
Copy link
Collaborator

Included in 1.16 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants