Invalid redirect to RedirectUrl on error during 2FA

[vladdy-moses]

Summary

CookieAuthenticationHandler redirects user to ReturnUrl path after successful sign-in event.
But it is not appropriate for TwoFactorUserIdCookie.

Motivation and goals

Sometimes something goes wrong after SignInManager<T>.PasswordSignInAsync with RequiresTwoFactor result.
And we want to render page with invalid ModelState, but ASP.NET Core redirects user to RedirectUri path.

Now in our projects we disable OnRedirectToReturnUrl event for IdentityConstants.TwoFactorUserIdScheme, but it looks like a crutch.

In scope

There is default ASP.NET Core app with Identity and enabled authentication and authorization.
The main page has [AuthorizeAttribute].
Also there is /Account/Login page (ex. Razor Page is used) with [AllowAnonymousAttribute].

If anonymous user opens our app, it will be redirected to '/Account/Login?RedirectUri=/` path.

After signing in by using SignInManager<T>.PasswordSignInAsync app is handling IdentityResult.
If result is RequiresTwoFactor our app make some logic (ex. form and send email for 2FA).
And if error is happened we should display it by using ModelState, for example.

But if path of sign-in page is default (defined here and here) default cookie authentication handler (here) make Context.Redirect to ReturnUrl.

It happens because SignInManager actually signed in IdentityConstants.TwoFactorUserIdScheme here and cookie authentication handler runs for that scheme.

I think default cookie config here and here should be changed.
For example we can make o.Events.OnRedirectToReturnUrl = _ => Task.CompletedTask; for disable redirection for 2fa auth scheme.

Out of scope

This proposal covers only default asp.net identity config.

Risks / unknowns

Perhaps in some applications such a redirection is valid.

Examples

Sample asp.net core 6 for minimal APIs was created here.
This app creates sample user with 2FA and tries to sign-in.
Also there are some workarounds for now.

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[adityamandaleeka]

Thanks for filing a detailed issue @vladdy-moses.

[HaoK]

This indeed looks like a bug, and your suggested fix of explicitly doing o.Events.OnRedirectToReturnUrl = _ => Task.CompletedTask for the two factor cookie looks reasonable.

We'll fix this in 8.0, are you interested in contributing a PR with this fix?

[vladdy-moses]

are you interested in contributing a PR with this fix?

I'm very appreciate your offer and already made PR #44435.

All tests are passed, but also I'd like to write new test for reproduce this issue problem.
Should I open a new issue for that?

[HaoK]

You can just add the test for this issue in that PR