aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 869 forks source link

SignInManager.PasswordSignInAsync will cause redirect (302) When 2FA is enabled. #1963

Closed alonstar closed 5 years ago

alonstar commented 5 years ago

Identity 2.1.3 & AspNetCore.App 2.1.3

I need to verify 2FA but signInManager.PasswordSignInAsync will cause Response.StatusCode = 302. It will redirect to home index.

How can I prevent it to do this?

Now I added HttpContext.Response.StatusCode = (int)HttpStatusCode.OK after PasswordSignInAsync to prevent it redirect.

blowdart commented 5 years ago

Where do you want it to go? After PasswordSigninAsync you need the 2fa verification page as well.

You'd check via

                var result = await _signInManager.PasswordSignInAsync(Input.Email, Input.Password, Input.RememberMe, lockoutOnFailure: true);
                if (result.Succeeded)
                {
                    _logger.LogInformation("User logged in.");
                    return LocalRedirect(Url.GetLocalUrl(returnUrl));
                }
                if (result.RequiresTwoFactor)
                {
                    return RedirectToPage("./LoginWith2fa", new { ReturnUrl = returnUrl, RememberMe = Input.RememberMe });
                }

And then in your 2fa page

            var user = await _signInManager.GetTwoFactorAuthenticationUserAsync();
            if (user == null)
            {
                throw new ApplicationException($"Unable to load two-factor authentication user.");
            }

            var authenticatorCode = Input.TwoFactorCode.Replace(" ", string.Empty).Replace("-", string.Empty);

            var result = await _signInManager.TwoFactorAuthenticatorSignInAsync(authenticatorCode, rememberMe, Input.RememberMachine);

            if (result.Succeeded)
            {
                _logger.LogInformation("User with ID '{UserId}' logged in with 2fa.", user.Id);
                return LocalRedirect(Url.GetLocalUrl(returnUrl));
            }
            else if (result.IsLockedOut)
            {
                _logger.LogWarning("User with ID '{UserId}' account locked out.", user.Id);
                return RedirectToPage("./Lockout");
            }
            else
            {
                _logger.LogWarning("Invalid authenticator code entered for user with ID '{UserId}'.", user.Id);
                ModelState.AddModelError(string.Empty, "Invalid authenticator code.");
                return Page();
            }
alonstar commented 5 years ago

@blowdart I have another issue to handle, so I need return "View" sometimes, not redirect. BTW, why can't keep 2fa in the same page? If I need to redirect, I also can use "RedirectToAction" or "Redirect" to do this, right? so I think "PasswordSignInAsync" should not to force to redirect.

alonstar commented 5 years ago

https://github.com/aspnet/Identity/issues/1543

There is someone has the same issue with me.~

blowdart commented 5 years ago

You generally don't return views after POSTs, Post/Redirect/Get (PRG) pattern is almost always used, because it stops duplication form submissions, hence our code above. If you want to vary from common practice you are, I'm afraid, on your own.

As for 2fa in the same page, well, what if the user doesn't have 2fa enabled, how do you know until you get a POST with the username?

alonstar commented 5 years ago

I understand Post/Redirect/Get (PRG) pattern is always used, but it's "always", not "must", I don't understand why it must require to redirect after post ? Redirect or not it's should depend on situation, If I don't want user to post again, there are many ways to do this thing, not just redirect.

I think every method should keep it to do a single thing or complete flow, after PasswordSignInAsync, there are always still something need to do.

Our user must to do something to active 2FA, and we require user must use 2FA, so after PasswordSignInAsync, if result is 2FA enabled but the other options are not active, I return a error message to tell user (so I don't want the page to redirect).

Or I need use PasswordSignInAsync on API, I need 200 not 302 as a result.

As many many something need to do, so I hope the PasswordSignInAsync not to cause 302.

blowdart commented 5 years ago

This is just how we do it. It's not going to change. You can consider replacing our code with the suggestions above.