brockallen / BrockAllen.MembershipReboot

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

emailIsUsername=true & requireAccountVerification="false" & IdentityServerV3 #541

Closed TheOtherOnes closed 8 years ago

TheOtherOnes commented 9 years ago

Brock,

I have read your explanations of the challenges with using emailIsUsername. I'm challenged with having to use this with IdentityServerV3 and requireAccountVerification=false. My company doesn't want to require new users to confirm during registration.

The problem occurs when the users wants to change their email and the username won't change until confirmed. In this situation, the user doesn't receive the verification email, so the username remains the original and the user can't login to IdentityServer with the new email address.

I thought I might be able to add code to the ChangeEmail controller to automatically confirm with userAccountService.VerifyEmailFromKey(), but I don't have a way get the key because it's not the VerificationKey in the database.

Is there a way I can force the verification? Is there a way I can force the confirmation email to be sent even though requireAccountVerification=false (because I assumed this only applied to registration)?

Thanks in advance.

Adam

TheOtherOnes commented 9 years ago

Ok, I figured out that the email confirmation does get sent, but it gets sent to the new email, not the old email.

Now I find it odd that, although I'm calling _userAccountService.ChangeEmailRequest, evt.GetType().Name is EmailChangedEvent. This means that the EmailChanged email is sent rather than the ChangeEmailRequest email. This is confusing because the process isn't really complete until confirmed and the username has been changed.

I do realize that the email has already been changed by the time the email is sent, but with emailIsUsername=true, it really isn't complete. If the verification email times out before the user confirms, they are stuck in limbo and can't get back.

Adam

brockallen commented 9 years ago

Sorry for the delay -- this got lost in the past month of me traveling.

So yea, I guess this might be an issue. Is the problem just the email notifications?

eric-swann-q2 commented 9 years ago

I have a related but slightly different issue:
I have RequireAccountVerification = false and am using the email as the username, although don't think it is important to this scenario. When the user attempts to reset their password the AccountService insists on confirming the account before allowing password reset. But I've already indicated that the account confirmation should not be required in the settings. Is this by design or just a miss?

I see the configuration is available already in the account service so the "Require Verification" setting could be checked as part of the condition for password reset. If that sounds ok, happy to do a pull request.

Best, Eric

brockallen commented 9 years ago

Yes, we always require a confirmed email before password reset is allowed -- it's just not secure otherwise. The RequireAccountVerification name is a bit misleading -- that just means if the user's email must be confirmed for them to login.

eric-swann-q2 commented 9 years ago

Cool thanks.

TheOtherOnes commented 8 years ago

So yea, I guess this might be an issue. Is the problem just the email notifications?

Sorry for my delay. Yes, it's the events that are raised that cause the emails to get sent that are confusing me. EmailChangedEvent vs. EmailChangeRequestedEvent. They don't seem to be aligned properly.