Emit more logging and errors around authentication scheme at startup

[silamon]

Issue / Steps to reproduce the problem

We are receiving NullReference exceptions because we do not fill in the UserInteractionOptions. Earlier in the v2.2.0 build, this was automatically filled with default values when the IdentityServerOptions is initialized. Now in the preview of v2.3.0, it looks like the default path is no longer filled in automatically. It remains furthermore null causing NullReference exceptions on other places.

Related: #2532

[brockallen]

Do you have a repro for this or a call stack? I can't seem to cause this NRE in our host with our samples.

[brockallen]

@silamon any update? if you can't help us repo this report, then there's nothing we can do to fix it.

[silamon]

Sorry for the late response. I reverted back to 2.2.0 after submitting the bug report. The exception no longer happens when I return to 2.3.0 preview now so I have not been able to reproduce this anymore. I can still remember that the NRE was caused on login. In LoginPageResult _options.UserInteraction.LoginUrl was null. Then a bit later it tries to invoke AddQueryString on null. Related code: https://github.com/IdentityServer/IdentityServer4/blob/dev/src/Endpoints/Results/LoginPageResult.cs#L61-L69.

[silamon]

I've been able to reproduce it now. That day I tried to set different authentication schemes.

This is the original code:
.AddAuthentication( IdentityServerAuthenticationDefaults.AuthenticationScheme )

Then I played around and used:

.AddAuthentication(options => {
    options.DefaultScheme = IdentityServerAuthenticationDefaults.AuthenticationScheme;
    options.DefaultAuthenticateScheme = IdentityServerAuthenticationDefaults.AuthenticationScheme;
    /* ... */ 
})

(Just for the clarification, I have combined API & IdentityServer in one project. Therefore I need authentication & authorization for the API, but not for the Identity Server. )

[brockallen]

Ok, that helps. I'll look into it.

[brockallen]

Can you show me your entire Startup that's not working?

[brockallen]

IdentityServerAuthenticationDefaults.AuthenticationScheme is "Bearer", mind you. So that's fundamentally wrong for the default scheme. That's not something that should have been working previously.

[brockallen]

And so to complete my thought... the default scheme needs to be your cookie scheme, or you can set the new CookieAuthenticationScheme on the IdentityServer AuthenticationOptions like this:

services.AddIdentityServer(options =>
{
   options.Authentication.CookieAuthenticationScheme = "YourCookieScheme";
})

[silamon]

Thanks for helping me out, but I've already found a solution now that both API & Auth are combined.

For this issue, it may be great if you could add an (better) exception which is given if the authentication scheme is not correctly set. It didn't really work in v2.2 but gave an exception after correct login: InvalidOperationException: The authentication handler registered for scheme 'Bearer' is 'IdentityServerAuthenticationHandler' which cannot be used for SignInAsync. The registered sign-in schemes are: idsrv, idsrv.external.. In v2.3, you get the NRE for the same code. In v2.2, it was filled in automatically and therefore giving no NRE.

If you don't feel like an exception is required, you may close the issue.

[brockallen]

This exception is thrown by the Microsoft plumbing, not us. I think what might be nice is if we check at startup if we can determine the default authentication scheme and if it is cookies and if not then we issue a warning/error in the logs.

[brockallen]

I added additional logging at startup to help with this issue. @leastprivilege please have a look.

[brockallen]

all done and reviewed

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.