brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

ChangeEmailRequest verification doesn't seem to be required #579

Closed TheOtherOnes closed 8 years ago

TheOtherOnes commented 8 years ago

When a user requests a change in email address (my controller calls ChangeEmailRequest()), they get an email sent to the new address, because the UserAccounts table now has their new address and their old address is lost. The email sent requests that they confirm the change, but it has already changed and there doesn't seem to be a way for them to undo the change, or for the system to undo it for them if they let the timeout expire.

In this case, what is the point of verification?

Thanks

Adam

brockallen commented 8 years ago

The expected/normal flow is this: When a request comes in the new email is stored in the state column, and so the old email is not lost. The email is not overwritten until the new email is confirmed.

If you're getting different behavior, then you might be using SetConfirmedEmail? Or somehow bypassing the expected workflow?

Also, there's a separate column/property to track if the email was confirmed. The confirmation set this.

TheOtherOnes commented 8 years ago

When you say "state column" do you mean VerificationStorage?

I think what I'm seeing is a side effect of RequireAccountVerification=false. Looks like you are updating the email address directly, yet all the verification fields are filled too.

            if (!Configuration.RequireAccountVerification)
            {
                Tracing.Verbose("[UserAccountService.ChangeEmailRequest] RequireAccountVerification false, changing email");
                account.IsAccountVerified = false;
                account.Email = newEmail;
                this.AddEvent(new EmailChangedEvent<TAccount> { Account = account, OldEmail = oldEmail, VerificationKey = key });
                Update(account);
            }

I don't suppose all the effects of RequireAccountVerification is documented anywhere. It looks to me like it does many things.

brockallen commented 8 years ago

Really sorry for letting these issues get so old without attention.

The intent with RequireAccountVerification was the prevent the user from logging in until they confirmed their email.