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

user.emailVerified is not set to false when user.email changes #4501

Closed
henrikperrochon opened this issue Jan 16, 2018 · 13 comments
Closed

user.emailVerified is not set to false when user.email changes #4501

henrikperrochon opened this issue Jan 16, 2018 · 13 comments
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@henrikperrochon
Copy link

henrikperrochon commented Jan 16, 2018

Issue Description

When a user has verified their email, and then change it, the field emailVerified is still true.

Steps to reproduce

One user set their email. user.email = aaa@aaa.aa;
They clicks on the link to verify the email. user.emailVerified = true;
They changes their email. user.email = bbb@bbb.bb; user.emailVerified = true;

Expected Results

I was expected emailVerified to become false again.

Environment Setup

  • Server
    • parse-server version 2.7.1

How to handle it

It might not be required by everyone, so I wanted to solve that issue inside the beforeSave of Parse.User, something like:

if (request.object.dirty('email') && !(request.original && request.original.get('email') === request.object.get('email'))) {
    request.object.set('emailVerified', false)
}

Unfortunately, the user.emailVerified field is protected, you need the masterKey to update that field. Maybe I have to call user.save({ emailVerified: false }, { useMasterKey: true }) inside user beforeSave but I don't like the idea (infinite loop). Maybe we could only require masterKey for setting emailVerified to true?

Thanks!

@montymxb
Copy link
Contributor

Hmm, this is interesting point. @flovilmart makes sense to reset verified when the email has been changed? Trying to think if something goofy would arise from this...

@flovilmart
Copy link
Contributor

Yes it makes total sense! We should do it

@montymxb montymxb added type:bug Impaired feature or lacking behavior that is likely assumed good first issue labels Jan 20, 2018
@paulovitin
Copy link
Contributor

@montymxb Can I take care of this?

@flovilmart
Copy link
Contributor

@paulovitin yes! Jump ahead!

@paulovitin
Copy link
Contributor

@flovilmart just one question, the user can change the email before of confirm the old email or not?

@montymxb
Copy link
Contributor

montymxb commented Jan 25, 2018

@paulovitin yep go for it! And yes, the user may change their email before confirming the existing one. A good example would be if you put the wrong email (or a non-existent email with a typo) the first time around. You would want to correct that even though you cannot necessarily confirm the existing email.

Another question would be whether we would to send another verification email automatically or manually (assuming email verification is enabled). On signup the email is sent automatically, if the email is subsequently changed, and the user is not informed to re-verify by another request, it could be problematic depending on what's controlled by emailVerified. The manual case would be up to the dev to decide.

@paulovitin
Copy link
Contributor

@montymxb look the test in PR, the problem cant be found. I create a test with the scenario and the test pass.

@Helmikku can you have a look in the PR?

@montymxb
Copy link
Contributor

montymxb commented Feb 1, 2018

@paulovitin no kidding...alright hold I'll take a look at your change.

@montymxb
Copy link
Contributor

montymxb commented Feb 1, 2018

It looks like the following test accounts for this behavior, but it doesn't set an email first and then set it again. However it does demonstrate that updating a user with a new email will setup the verification process again.

@paulovitin I think it would be prudent to put up a PR for a test that explicitly checks for this behavior when an email has already been set and verified beforehand. If you have time you can base it off the test you put together already, there are some things I want to correct but it looks like a good start.

montymxb pushed a commit that referenced this issue Feb 1, 2018
…existing (#4532)

* Create a test to check the issue #4501
* Check if after the user confirms the email and change the email again a new verification email is sent
* Change the spec text to requested in PR
@montymxb
Copy link
Contributor

montymxb commented Feb 1, 2018

Closing this out as #4532 has been merged. @Helmikku if your issue persists let us know. Although we didn't introduce any fix specifically the supplemental test we've added indicates that emailVerified is properly cleared when updating from an existing email that was also verified.

@montymxb montymxb closed this as completed Feb 1, 2018
@henrikperrochon
Copy link
Author

Hi @montymxb

Thanks for your feedback. For instance, if I update an email from the dashboard, the emailVerified field is still true (I refreshed the page to be sure the data is up to date):

screenshot-2018-2-2 parse dashboard
screenshot-2018-2-2 parse dashboard 1

Regarding my setup, I've compared with the test @paulovitin provided and since I'm using my own verification process, I don't have those field in my init json:

  •  verifyUserEmails: true,
    
  •  emailAdapter: emailAdapter,
    
  •  emailVerifyTokenValidityDuration: 5, // 5 seconds
    

Isn't verifyUserEmails dedicated to send email? Or should it be enabled even if I use my own flow?
I send a link with a custom token, that I use with a cloud function to update emailVerified (with useMasterKey: true).

@montymxb
Copy link
Contributor

Ahh... If you're using your own flow and not using the internal method I suspect it won't set the verified field back for you. In your setup you're going to have to do this yourself unfortunately, unless you decide to utilize the built in method.

@RaschidJFR
Copy link
Contributor

The emvailVerified field changes to false when modifiend the email field from Parse.User.save({email}). But shouldn't this not change until the email is verified? Otherwise, the user could get locked out (if your app's logic prevents unverified emails from logging in for example) without a chance to reset their password!

UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this issue Mar 21, 2020
…existing (parse-community#4532)

* Create a test to check the issue parse-community#4501
* Check if after the user confirms the email and change the email again a new verification email is sent
* Change the spec text to requested in PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Development

No branches or pull requests

5 participants