AuthN: Use single scheme as DefaultScheme

[HaoK]

Note this is a breaking change, part of #42481

Automatically use a single authN scheme as the defaultScheme when only one is registered.

This can be disabled via: AuthenticationOptions.DisableAutoDefaultScheme = true;

[HaoK]

I started down the path of PostConfigureOptions but that didn't work well at all. Seems safest to just respect the value of this flag in the actual scheme provider implementation which is the actual source of truth for what schemes are valid.

[HaoK]

These tests are only testing very specific paths, setting the default scheme causes some of the other schemes to now have values which causes exceptions in these tests which are unrelated to what they are trying to test, easiest to just disable the new behavior

[Tratcher]

Please post a draft of the breaking change announcement here as a comment for review.

[HaoK]

Breaking change draft:

Previously, DefaultScheme needed to be explicitly set, even when there was only a single authentication scheme registered. We've decided to change the behavior of the default IAuthenticationSchemeProvder implementation to use a single authentication scheme as the default, when no DefaultScheme is specified explicitly.

This allows skipping specifying the DefaultScheme in your services configuration, i.e. AddAuthentication("canSkipIfSingleScheme") for apps that only have a single authentication scheme.

There is no change in behavior when a DefaultScheme is specified explicitly, or when more than one authentication scheme is registered. To opt out of this new behavior, set AuthenticationOptions.DisableAutoDefaultScheme = true

[Tratcher]

Would we consider making this an AppContext switch for one release to see if anybody ever needs it?

[HaoK]

Is that in practice better than just having a obscure public flag that no one uses?

[Tratcher]

The flag lives forever, even if nobody needs it. A switch we can remove if nobody uses it.

@DamianEdwards ?

[DamianEdwards]

It's true we can avoid the public API addition if it turns out nobody uses the AppContext switch for a release. I'm supportive of changing this to a switch instead and we can always add the option if we decide it's worthwhile.

[HaoK]

Sounds fine as long as we have a clear idea of how to make this determination in 8.0, or are we just going to remove it and see if anyone complains in 8?

[halter73]

We can do both an internal option for testing and an appcontext switch for everyone else. For example:

aspnetcore/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs

Lines 160 to 176 in d96a100

	/// <summary>
	/// Internal AppContext switch to toggle the WebTransport and HTTP/3 datagrams experiemental features.
	/// </summary>
	private bool? _enableWebTransportAndH3Datagrams;
	internal bool EnableWebTransportAndH3Datagrams
	{
	get
	{
	if (!_enableWebTransportAndH3Datagrams.HasValue)
	{
	_enableWebTransportAndH3Datagrams = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.Experimental.WebTransportAndH3Datagrams", out var enabled) && enabled;
	}
	return _enableWebTransportAndH3Datagrams.Value;
	}
	set => _enableWebTransportAndH3Datagrams = value;
	}

[HaoK]

Cool that's not too bad then as the code will basically stay the same in this PR, just an additional app context check in the get for the options property

[HaoK]

Note I still think this is a net negative, as now we have internals visible to 3 more assemblies, when we could have just had this be a public bool

[DamianEdwards]

I'll leave it you ya'll to decide, but I tend to agree with @HaoK in this case that it seems like a lot of trouble to avoid a single public bool property.

[Tratcher]

Its unusual for the options class and the consuming implementation to be in different assemblies, that does make this messier. I'm not sure which is worse, IVT or a useless property 😢.

, at least the IVT can be removed later.

[ghost]

@HaoK, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

[adityamandaleeka]

Tentatively adding blog-candidate label to this. Probably at least worth including as part of a section on the various auth changes that went into Preview 7?