Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Asp.net core identity change username/email

The default identity change username/email with confirmation logic doesn't make sense.

  • Set app with require email confirmation.
  • Set require confirmed email to sign in.
  • User then changes email, enters email incorrectly, logs out.
  • Now the user is locked out. Email has changed but requires confirmation to sign in and no email confirmation link because address entered incorrectly.

Have I setup my application wrong or did Microsoft not design Identity very well?

  public async Task<IActionResult> OnPostAsync()
        {
            if (!ModelState.IsValid)
            {
                return Page();
            }

            var user = await _userManager.GetUserAsync(User);
            if (user == null)
            {
                return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
            }

   //...

      var email = await _userManager.GetEmailAsync(user);
            if (Input.Email != email)
            {
                var setEmailResult = await _userManager.SetEmailAsync(user, Input.Email);
                if (!setEmailResult.Succeeded)
                {
                    var userId = await _userManager.GetUserIdAsync(user);
                    throw new InvalidOperationException($"Unexpected error occurred setting email for user with ID '{userId}'.");
                }
                StatusMessage = "<strong>Verify your new email</strong><br/><br/>" +
                    "We sent an email to " + Input.Email +
                    " to verify your address. Please click the link in that email to continue.";
            }

  //...


        await _signInManager.RefreshSignInAsync(user);

        return RedirectToPage();
    }
like image 456
pete Avatar asked Jan 02 '23 10:01

pete


2 Answers

Your issue is using SetEmailAsync for this purpose. That method is intended to set an email for a user when none exists currently. In such a case, setting confirmed to false makes sense and wouldn't cause any problems.

There's another method, ChangeEmailAsync, which is what you should be using. This method requires a token, which would be obtained from the email confirmation flow. In other words, the steps you should are:

  1. User submits form with new email to change to
  2. You send a confirmation email to the user. The email address the user is changing to will need to be persisted either in the confirmation link or in a separate place in your database. In other words, the user's actual email in their user record has not changed.
  3. User clicks confirmation link in email. You get the new email address they want to change to either from the link or wherever you persisted it previously
  4. You call ChangeEmailAsync with this email and the token from from the confirmation link.
  5. User's email is now changed and confirmed.

EDIT

FWIW, yes, this appears to be an issue with the default template. Not sure why they did it this way, since yes, it very much breaks things, and like I said in my answer, ChangeEmailAsync exists for this very purpose. Just follow the steps I outlined above and change the logic here for what happens when the user submits a new email address via the Manage page.

EDIT #2

I've filed an issue on Github for this. I can't devote any more time to it at the moment, but I'll try to submit a pull request for a fix if I have time and no one else beats me to it. The fix is relatively straight-forward.

EDIT #3

I was able to get a basic email change flow working in a fork. However, the team has already assigned out the issue and seem to be including it as part of a larger overhaul of the Identity UI. I likely won't devote any more time to this now, but encourage you to follow the issue for updates from the team. If you do happen to borrow from my code to implement a fix now, be advised that I was attempting to create a solution with a minimal amount of entropy to other code. In a real production app, you should persist the new email somewhere like in the database instead of passing it around in the URL, for example.

like image 188
Chris Pratt Avatar answered Jan 21 '23 13:01

Chris Pratt


As already identified, the template definitely provides the wrong behaviour. You can see the source for the template in the https://github.com/aspnet/Scaffolding repo here.

I suggest raising an issue on the GitHub project so this is changed. When the templates are updated, they'll no doubt have to account for both the case when confirmation is enabled and when it's not. In your case, you can reuse the logic that already exists in OnPostSendVerificationEmailAsync() relatively easily.

A more general implementation would look something like this:

public partial class IndexModel : PageModel
{
    // inject as IOptions<IdentityOptions> into constructor
    private readonly IdentityOptions _options;

    // Extracted from OnPostSendVerificationEmailAsync()
    private async Task SendConfirmationEmail(IdentityUser user, string email)
    {
        var userId = await _userManager.GetUserIdAsync(user);
        var code = await _userManager.GenerateEmailConfirmationTokenAsync(user);
        var callbackUrl = Url.Page(
            "/Account/ConfirmEmail",
            pageHandler: null,
            values: new { userId = userId, code = code },
            protocol: Request.Scheme);
        await _emailSender.SendEmailAsync(
            email,
            "Confirm your email",
            $"Please confirm your account by <a href='{HtmlEncoder.Default.Encode(callbackUrl)}'>clicking here</a>.");
    }

    public async Task<IActionResult> OnPostAsync()
    {
        //... Existing code

        var email = await _userManager.GetEmailAsync(user);
        var confirmationEmailSent = false;
        if (Input.Email != email)
        {
            if(_options.SignIn.RequireConfirmedEmail)
            {
                // new implementation
                await SendConfirmationEmail(user, Input.Email);
                confirmationEmailSent = true;
            }
            else
            {
                // current implementation
                var setEmailResult = await _userManager.SetEmailAsync(user, Input.Email);
                if (!setEmailResult.Succeeded)
                {
                    var userId = await _userManager.GetUserIdAsync(user);
                    throw new InvalidOperationException($"Unexpected error occurred setting email for user with ID '{userId}'.");
                }
            }
            var setEmailResult = await _userManager.SetEmailAsync(user, Input.Email);
            if (!setEmailResult.Succeeded)
            {
                var userId = await _userManager.GetUserIdAsync(user);
                throw new InvalidOperationException($"Unexpected error occurred setting email for user with ID '{userId}'.");
            }
        }

        // existing update phone number code;

        await _signInManager.RefreshSignInAsync(user);
        StatusMessage = confirmationEmailSent 
            ? "Verification email sent. Please check your email."
            : "Your profile has been updated";
        return RedirectToPage();
    }


    public async Task<IActionResult> OnPostSendVerificationEmailAsync()
    {
        if (!ModelState.IsValid)
        {
            return Page();
        }

        var user = await _userManager.GetUserAsync(User);
        if (user == null)
        {
            return NotFound($"Unable to load user with ID '{_userManager.GetUserId(User)}'.");
        }

        var email = await _userManager.GetEmailAsync(user);
        await SendConfirmationEmail(user, email);

        StatusMessage = "Verification email sent. Please check your email.";
        return RedirectToPage();
    }
}
like image 22
Sock Avatar answered Jan 21 '23 14:01

Sock