4

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();
    }
Improve this question
11
  • Can you edit your question and include the code you are using for the change email controllers? You should not actually save the changed email address until the user confirms it. Commented Jan 10, 2019 at 13:14
  • @DaImTo I've updated my question. Its the default scaffolding for index.cshtml page in identity/pages/account/manage. Commented Jan 10, 2019 at 13:33
  • The default from where? I would say thats wrong as its not even sending a conformation code with it. Commented Jan 10, 2019 at 14:02
  • Create new asp.net core web application (asp.net core 2.2, mvc). Add Identity scaffolding. In Areas/identity/pages/account/manage/Index.cshtml.cs Commented Jan 10, 2019 at 14:18
  • That's kind of my point, it's not doing what you would expect it to do. I understand that the templates are just a starting point to a project, but I would expect the user management/profile/identity code to be a bit more polished. Especially considering asp.net is 17 years old... Commented Jan 10, 2019 at 14:24

2 Answers 2

Reset to default
11

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

What is this ChangeEmailAsync method you speak of, I don't see it?
It's off UserManager.
It doesn't appear in v2.2.2, what version are you looking at?
It's in all versions. It's always been there. learn.microsoft.com/en-us/dotnet/api/…
Interesting as it's not in mine. Wonder if it's an issue with using UserManager<TUser, TKey> as I'm using a long instead of a string for TKey?
|
1

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();
    }
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.