damienbod / AspNetCoreID4External

external OpenID Connect Login to IdentityServer and ASP.NET Core with AAD
https://damienbod.com/2019/05/17/updating-microsoft-account-logins-in-asp-net-core-with-openid-connect-and-azure-active-directory/
MIT License
79 stars 26 forks source link

TwoFactorSignIn overwriting IdP for ExternalProviders #17

Open zyofeng opened 3 years ago

zyofeng commented 3 years ago

At the moment, if an external user is flagged as requiring 2FA, the controller redirects the user to SendCode/VerifyCode flow, which ends with the following line of code

https://github.com/damienbod/AspNetCoreID4External/blob/3fa199ad4c1b8cb5379cea27c9b3af0d638bedbb/src/IdentityServerWithAspNetIdentity/Controllers/AccountController.cs#L617

This overwrites the IdP claim against the ClaimsPrincipal with "Local", which prevents upstream logout here https://github.com/damienbod/AspNetCoreID4External/blob/3fa199ad4c1b8cb5379cea27c9b3af0d638bedbb/src/IdentityServerWithAspNetIdentity/Controllers/AccountController.cs#L198

The workaround I have without having to rewrite TwoFactorSignInAsync is this, I am wondering if there is a more elegant solution?

var result = await _signInManager.TwoFactorSignInAsync(model.Provider, model.Code, model.RememberMe, model.RememberBrowser).ConfigureAwait(false);

//This is a workaround to override the Idp if it's an external provider if (result.Succeeded) { var info = await _signInManager.GetExternalLoginInfoAsync().ConfigureAwait(false); if (info != null) return await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, model.RememberMe, true).ConfigureAwait(false); return result; }

damienbod commented 3 years ago

@zyofeng I think this could be improved by using different schemes. I would need to look more into detail to understand this better.

Greetings Damien