Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identity: Existing "Email Confirmation" token invalidated if Two-Factor is enabled before verifying the account #32681

Closed
alistairjevans opened this issue May 14, 2021 · 2 comments
Assignees
Labels
area-identity Includes: Identity and providers ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Milestone

Comments

@alistairjevans
Copy link

alistairjevans commented May 14, 2021

Describe the bug

If I generate an Email Confirmation token (and send an appropriate email to the user containing a link with that token), but the user enables 2FA before clicking on the link, the Email Confirmation token subsequently fails validation.

When a new user registers, we send an out-of-band email to let the new user confirm their email address, by creating an email confirmation token using the standard UserManager.GenerateEmailConfirmationTokenAsync method and including that token in a link in said email.

We automatically sign the user in immediately upon registering, without needing them to confirm their email first. This is not the same as the out-of-the-box Identity behaviour that comes with the templates, where you cannot login until you have verified your email.

It is pretty common for our users to go straight to account settings after registering, before verifying their email address, where they enable Two Factor authentication. They then try to click the link we sent them when they go look at their inbox, and the token cannot be verified.

This user flow certainly appears to be fairly normal from my perspective?

I've taken a look at the code, and what happens is:

  • GenerateEmailConfirmationTokenAsync indirectly calls the DataProtectorTokenProvider to generate a new email token.
  • The DataProtectorTokenProvider embeds the user's SecurityStamp in the generated token.
  • we send the token to the user
  • When they enable Two Factor, their account's SecurityStamp is updated.
  • user clicks the verify link
  • The DataProtectorTokenProvider checks the stamp in the token, which no longer matches the users new SecurityStamp.

To workaround this issue, I have created a new implementation of IUserTwoFactorTokenProvider that excludes the SecurityStamp from the generated token, and used that for the EmailConfirmationTokenProvider.

Adding a custom provider may in fact be the recommended solution to this problem, but the default behaviour seemed unexpected enough that I thought I'd raise it. Seems risky for a number of people to reimplement the token provider for a relatively standard case?

To Reproduce

  1. Generate an EmailConfirmation token.
  2. Enable Two Factor on a account.
  3. Attempt to Verify the Email Confirmation token, which fails because the Security Stamp has changed.
// Simplified example
public class IndexModel : PageModel
{
    private readonly UserManager<IdentityUser> _userManager;
    private readonly ILogger<IndexModel> _logger;

    public IndexModel(UserManager<IdentityUser> userManager, ILogger<IndexModel> logger)
    {
        _userManager = userManager;
        _logger = logger;
    }

    public async Task OnGet()
    {
        var user = new IdentityUser
        {
            UserName = "test@gmail.com",
            Email = "test@gmail.com"
        };

        await _userManager.CreateAsync(user, "12345678");

        var emailConfirmToken = await _userManager.GenerateEmailConfirmationTokenAsync(user);

        await _userManager.SetTwoFactorEnabledAsync(user, true);

        var verifyTokenResult = await _userManager.ConfirmEmailAsync(user, emailConfirmToken);

        // Succeeded is false: Invalid Token.
        if (!verifyTokenResult.Succeeded)
        {
            throw new Exception("Failed validation");
        }
    }
}

Further technical details

  • ASP.NET Core version : Identified on netcoreapp3.1, but issue remains in .net5
@Pilchie Pilchie added the area-identity Includes: Identity and providers label May 14, 2021
@mkArtakMSFT mkArtakMSFT added this to the Discussions milestone May 20, 2021
@blowdart
Copy link
Contributor

This is unfortunate but it's a design decision. Tokens are generated from a user's security stamp. When certain operations that affect the identity of the user the stamp is swapped out to for a new one, to invalidate cookies and force logouts. Changing two factor status is one of these actions.

As the user can send another confirmation email with the new value, and changing the flow so the stamp isn't invalidated has security issues it's not something we'd look at changing.

@blowdart blowdart added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label May 20, 2021
@ghost ghost added the Status: Resolved label May 20, 2021
@ghost
Copy link

ghost commented May 21, 2021

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed May 21, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 20, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-identity Includes: Identity and providers ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants