brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
743 stars 239 forks source link

ChangeEmailRequest should change username in case of EmailIsUsername and not RequireAccountVerification #704

Open HenkHui opened 7 years ago

HenkHui commented 7 years ago

The ChangeEmailRequest checks the RequireAccountVerification config value. If no verification is required the e-mailaddress is changed without sending a verification request first, which is fine. However when the e-mailaddress is also the username, the username is not changed by this method. This leads to unexpected behavior. Because when the account was created the user could login with an unverified username (=e-mailaddress) but when we want to change the username (which needs to be done through ChangeEmailRequest because ChangeUsername cannot be used when EmailIsUsername) the verification must be done in order for the change to take effect. Why? :-)

brockallen commented 7 years ago

IIRC we don't want to trust the new email until the new email has been confirmed. If you change it prior to that then the account is in an inconsistent state.

HenkHui commented 7 years ago

Thanks for the quick response Brock. The e-mail is changed and the account state is set to not verified:

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

So the state of the account is the same as it would be when you created it with the RequireAccountVerification set to false, however the only thing off is the username.

Even more, the state of the account is now inconsistent because the username and the e-mailaddress are not the same despite of setting EmailIsUsername to true.