Fix external login flow for failed/partial authentication

[Eilon]

Original bug: aspnet/Identity#915

We want to make the change described in option (3) in @Tratcher 's comment at aspnet/Identity#915 (comment).

Specifically, if the user attempts to use an OAuth login and they still have the external login cookie, the template code will automatically pick up where it left off and complete the flow.

[Eilon]

We can have @troydai do the fix to the templates.

[mikes-gh]

We need to account for Identity.External being a different provider.
@Tratcher solution didn't allow for this.
TODO : refactor out repeated code

// POST: /Account/ExternalLogin
[HttpPost]
[AllowAnonymous]
[ValidateAntiForgeryToken]
public async Task<IActionResult> ExternalLogin(string provider, string returnUrl = null)
{
    var info = await _signInManager.GetExternalLoginInfoAsync();

    // No External Cookie (normal flow)
    if (info == null)
    {
        var redirectUrl = Url.Action("ExternalLoginCallback", "Account", new { ReturnUrl = returnUrl });
        var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl);
        return new ChallengeResult(provider, properties);
    }
    else
    {

        // External Cookie present same provider
        if (info.LoginProvider == provider)
        {
            return RedirectToAction(nameof(AccountController.ExternalLoginCallback), "Account");
        }

        // External Cookie present different provider
        else
        {
            await _signInManager.SignOutAsync();
            return RedirectToAction(nameof(AccountController.ExternalLoginRedirect), "Account", new { provider = provider, returnUrl = returnUrl });
        }
    }   
}

[HttpGet]
[AllowAnonymous]
public IActionResult ExternalLoginRedirect(string provider, string returnUrl = null)
{
    var redirectUrl = Url.Action("ExternalLoginCallback", "Account", new { ReturnUrl = returnUrl });
    var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl);
    return new ChallengeResult(provider, properties);
}

[troydai]

Thanks, @mikes-gh . A good point. I'll make sure different provider scenario is handled.

[Tratcher]

RE: Challenging for provider A does not currently work if you're already logged in with provider B and they share the same cookie: aspnet/Security#859

[troydai]

By the way, I'll also take a loot into issue aspnet/Security#859 as well. Challenging the auth if the provider is changed can be another approach.

[troydai]

Prototype v1

        [HttpPost]
        [AllowAnonymous]
        [ValidateAntiForgeryToken]
        public async Task<IActionResult> ExternalLogin(string provider, string returnUrl = null)
        {
            var info = await _signInManager.GetExternalLoginInfoAsync();
            if (info != null)
            {
                if (string.Equals(info.LoginProvider, provider))
                {
                    // There was a successful external log in from the same log in provider.
                    // Skip challenging and redirect to next step.
                    return RedirectToAction(nameof(ExternalLoginCallback));
                }
                else
                {
                    // There was a successful external log in from a different login provider.
                    // Sign out from the external log in provider
                    await _signInManager.SignOutAsync();

                    return RedirectToAction(
                        nameof(ExternalLoginChallenge), "Account",
                        new { provider = provider, returnUrl = returnUrl });
                }
            }

            return ExternalLoginChallenge(provider, returnUrl);
        }

        [HttpGet]
        [AllowAnonymous]
        public IActionResult ExternalLoginChallenge(string provider, string returnUrl = null)
        {
            var redirectUrl = Url.Action("ExternalLoginCallback", "Account", new { ReturnUrl = returnUrl });
            var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl);
            return Challenge(properties, provider);
        }

[troydai]

/cc @HaoK

[Tratcher]

Open questions:

• Is the signout redirect from Post to Get safe?
• Can we avoid the signout & redirect by fixing Authorize(Github) may return a Facebook user Security#859?

[HaoK]

My feeling is still that we should add a switch to turn off the new behavior we added instead of forcing this extra redirect, the flow is complicated enough without an additional redirect.

The AuthenticateUsingSignInScheme behavior is totally not desired in the multiple shared sign in cookie scenario, if we added an opt-out, then we'd only need to update the documentation when adding ExternalLogins to include setting this false.

We can actually already detect this multiple shared sign in scheme scenario already. If the Shared SignInScheme is set (which identity does by default) https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication/SharedAuthenticationOptions.cs

An alternative clean fix then could be to add the new AuthenticateUsingSignInScheme flag to SharedAuthenticationOptions as a bool?, update the RemoteMiddleware to also consume the new shared setting and then identity could just turn this off as part of its default setup. Existing apps could also opt out of this behavior by flipping that to off as well.