DisplayName in new AuthN Plumbing?

[leastprivilege]

How do I set it? It's gone from the options class.

[Tratcher]

Security/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs

Line 37 in ff9f145

public virtual AuthenticationBuilder AddScheme<TOptions, THandler>(string authenticationScheme, string displayName, Action<TOptions> configureOptions)

Security/src/Microsoft.AspNetCore.Authentication/AuthenticationServiceCollectionExtensions.cs

Line 50 in df325de

public static IServiceCollection AddScheme<TOptions, THandler>(this IServiceCollection services, string authenticationScheme, string displayName, Action<AuthenticationSchemeBuilder> configureScheme, Action<TOptions> configureOptions)

@HaoK is this exposed anywhere else? E.g. I don't see a way to change it for Google, OIDC, etc..

[leastprivilege]

but not here

https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectExtensions.cs#L20

[Tratcher]

It's set here to match the scheme name: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectExtensions.cs#L23
Needs an overload to set the display name.

[HaoK]

Yeah we don't have those overloads in the security layer right now, so you'd have to drop down and use the lower overload to tweak the display name

[kevinchalet]

If you decide to fix that in 2.0 (and I personally hope you will), please also fix aspnet/IISIntegration#391 (I'm fine submitting a PR if you prefer).

[HaoK]

Doubt we are doing that now, since its a non breaking change in 2.1

You can probably workaround this via directly tweaking the DisplayName via AuthenticationOptions.SchemeMap:

services.Configure<AuthenticationOptions>(o => o.SchemeMap["OIDC"].DisplayName = whatever

[leastprivilege]

That's ridiculous.

[kevinchalet]

Doubt we are doing that now, since its a non breaking change in 2.1

Well, it automatically means waiting maybe 5 or 6 months more before moving to 2.x.
And while you can indeed work around this limitation with the "classical" handlers, it's not possible with the host handlers.

[HaoK]

What are you trying to do with display name that is so critical? Check if its null?

[kevinchalet]

@HaoK aspnet/IISIntegration#391 (comment)

[HaoK]

So you can just write your own filtering for which handlers to display for the external schemes, you don't have to use DisplayName != null as your condition. Why don't you just merge the list you have with the Windows scheme?

[HaoK]

You are able to inspect the schemes yourself via IAuthenticationSchemeProvider, just compute the display list using that directly...

[HaoK]

Its up to the app to decide which schemes are appropriate anyways, we wrote some sugar (that I think lives in Identity?) that filters the schemes using DisplayName != null, but its just sugar

[kevinchalet]

Sure, why not reinventing the wheel and using a hardcoded display name in your views when you could simply rely on SignInManager.GetExternalAuthenticationSchemesAsync(), that does the right thing for you and is used in the default templates...

[HaoK]

Well, you can just wait til 2.1 as you said if you prefer that instead of using the workaround

[kevinchalet]

@HaoK maybe I wasn't clear enough: you CAN'T use the workaround you suggested with the host handlers due to the way you've decided to register them (in the IIS middleware constructor... dynamically ).

[HaoK]

The sugar is not meant to the final arbiter of what schemes should be displayed, that is template sugar for the Individual auth template basically. I never liked having that in the framework, but it is what it is. External clearly doesn't hosts as written either.

[HaoK]

The workaround for your case is to not use GetExternalAuthenticationSchemesAsync and compute that list yourself.

[HaoK]

Again, all that method does is:

var schemes = await _schemes.GetAllSchemesAsync();
return schemes.Where(s => !string.IsNullOrEmpty(s.DisplayName));

You really don't gain very much by calling it :p

[kevinchalet]

Okay so @leastprivilege was right. Arguing so hard for a zero-risk change that will take 10 minutes to do is truly ridiculous.

[kevinchalet]

You really don't gain very much by calling it :p

That's not the problem. The problem is that you can't use await _schemes.GetAllSchemesAsync(); as the display name will always be null. You'll have to hardcode it or import it from somewhere else, which makes no sense.

[HaoK]

Doesn't matter how long the fix takes, bar is higher than this bug, shipstoppers only.

[kevinchalet]

Updating the display name associated with the host handlers was possible in 1.x. Since it's no longer possible in 2.x, that's a regression. Does that qualify as a shipstopper?

services.Configure<IISOptions>(options =>
{
    options.AuthenticationDescriptions.Clear();

    options.AuthenticationDescriptions.Add(new AuthenticationDescription
    {
        AuthenticationScheme = IISDefaults.Negotiate,
        DisplayName = "Intranet"
    });

    options.AutomaticAuthentication = false;
});

[HaoK]

Most apps do not have a dynamic list of schemes, most have a couple of schemes they want to show, it really doesn't seem the end of the world for them to either:

• use the sugar, and tweak (hardcode "Windows" in your case).
• Or hardcode the entire list you return

[HaoK]

If there was no way for you to workaround this in your app, you'd have a stronger argument :)

[kevinchalet]

Most apps do not have a dynamic list of schemes, most have a couple of schemes they want to show

Then you should strongly consider revisiting how you handle that in 2.x/3.x because even if you don't like it, setting the display name in the options and using SignInManager.GetExternalAuthenticationSchemesAsync() was - and still is - the "official" way to do that.

But okay, point taken. I'll send a PR against the release branch as suggested by @Tratcher and try to bribe the guy in charge of approving the late PRs...

[HaoK]

Yeah this is a case of sugar overload in my mind. Those 2 lines should have just lived in the app, I tried to get rid of the concept entirely of DisplayName, but we couldn't come up with anything better, and we just ran out of time to spend on it so we brought it back late in preview 2 for the templates