Skip to content

Conditional email verification #7144

New issue

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

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

Already on GitHub? # to your account

Closed
3 tasks done
Samigos opened this issue Jan 26, 2021 · 20 comments · Fixed by #8425
Closed
3 tasks done

Conditional email verification #7144

Samigos opened this issue Jan 26, 2021 · 20 comments · Fixed by #8425
Labels
bounty:$250 Bounty applies for fixing this issue (Parse Bounty Program) state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature

Comments

@Samigos
Copy link

Samigos commented Jan 26, 2021

New Feature / Enhancement Checklist

Current Limitation

There are occasions where there are multiple groups of users in an app and email verification is needed only to some of them! Right now, Parse Server offers an option of enabling email verification upon #, but it becomes unconditional... it either works for everyone, or it does not at all.

Feature / Enhancement Description

To address this limitation, I think there could be a Boolean parameter at the # function that indicates whether email verification is required, or not.

Also, see initial forum post.

@mtrezza
Copy link
Member

mtrezza commented Jan 27, 2021

Thanks for suggesting.

I added the link to your forum post for reference.

To address this limitation, I think there could be a Boolean parameter at the # function that indicates whether email verification is required, or not.

There may also be other places where a change is necessary, for example on login. I'm not sure this should be an option in the sign-up method, because a malicious user could just change that option and do a REST sign-up to circumvent the restriction. Therefore I think a solution that is completely controlled server side would make more sense.

Since you already talk about "multiple groups of users", maybe this could be depending on the Role a user is assigned, but there might be simpler approaches.

Do you want to look into the email verification mechanism (mainly in UserController) in Parse Server and suggest how this could be done?

@parse-community parse-community deleted a comment from erencihangir Jan 27, 2021
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
@mtrezza
Copy link
Member

mtrezza commented Oct 9, 2022

Possible approaches to discuss:

  • a) Add a parameter to the beforeSave / before# trigger that includes a parameter to determine there whether a user requires email verification.
  • b) Extend the currently existing Parse Server option verifyUserEmails to also accept a function (instead of only a boolean value); if a function is set, use the return value of that function (true/ false) to determine per-user whether email verification is needed; the function should provide the full request object, similar to what a trigger would provide.

To extend the existing _User.emailVerified field, we could adopt the following approach:

  • field is undefined: email doesn't require verification; this is the same value as when the email verification requirement is turned off in the Parse Server option
  • field is false: email requires verification but isn't verified yet (or should it always check the function to determine whether verification is needed?, this makes the condition dynamic beyond an initial determination)
  • field is true: email is verified

@mtrezza mtrezza added the bounty:$100 Bounty applies for fixing this issue (Parse Bounty Program) label Oct 10, 2022
@mtrezza
Copy link
Member

mtrezza commented Oct 17, 2022

@mstniy What do you think about option (a), would that solve #8230?

If that works for you, we can look into how to implement it. If the suggestion above to extend the existing _User.emailVerified field may be a breaking change, we can look for a different implementation.

@mstniy
Copy link
Contributor

mstniy commented Oct 17, 2022

@mtrezza Could you elaborate (a) further?
The approach we have implemented in #8231 effectively adds a parameter to the # function, after all.

@mtrezza
Copy link
Member

mtrezza commented Oct 17, 2022

adds a parameter to the # function

That has been suggested by @Samigos and that is not an approach we'd pursue. It allows the client to determine whether an email needs to be verified. Only the server should have that authority. If you want to let the client decide you could use the context container so tell the server how it should behave. By doing so you of course accept the risk that a user may manipulate that option beyond your control.

Could you elaborate (a) further?

You can currently use the beforeSave and beforeLogin Cloud Code trigger to intercept the process when a user tries # for a new account or log into an existing account. In that trigger, we could add a parameter that you could manipulate to determine whether the user requires email verification.

For example:

Parse.Cloud.beforeSave(Parse.User, async req => {
  console.log(req.requireEmailVerification); // prints "true" or "false" depending on the Parse Server option setting
  req.requireEmailVerification = true; // override Parse Server option and require email verification for the user
  req.requireEmailVerification = false; // override Parse Server option and don't require email verification for the user
});

@mstniy
Copy link
Contributor

mstniy commented Oct 17, 2022

It allows the client to determine whether an email needs to be verified. Only the server should have that authority.

The changes in the PR ignore the value of emailVerified unless the requester is the master key. Please refer to the PR description, as well as the new test and the relevant lines of code

@mtrezza
Copy link
Member

mtrezza commented Oct 17, 2022

Equipping a client with the master key is something we do not advice, so this becomes a non-feature for most applications. Since we don't want to provide the same feature in multiple ways, we'd look for a solution that is universal and usable for most applications. I've added an example to maybe give a better idea of (a). Do you think that would cover your required feature?

@mstniy
Copy link
Contributor

mstniy commented Oct 17, 2022

The clients do not have the master key, so they won't have the option to bypass the verification.
Business logic running as a cloud function, or in another trusted application, can use the master key to bypass the verification.

I don't immediately see how (a) would be useful to us, as the information about whether the email verification should be bypassed or not would not be available to the trigger, but to the party issuing the request (with the master key).

@mtrezza
Copy link
Member

mtrezza commented Oct 17, 2022

I had another look at your PR. If I understand it correctly, you want to set the user email to be verified, without the user verifying it. But in the issue you also write:

We would like the master key to be able to bypass email verification, suppress the sending of the verification email

That is something that seems to be related to this feature here. With (a) you would disable the email verification for the user (i.e. suppress sending of the verification email), then in Cloud Code you would use the master key to set emailVerified: true as if the user had actually verified the email.

So the scope of #8230 is actually 2 functionalities:

  • suppress sending of email (this issue here)
  • manipulate emailVerified field (not within the scope of this issue here)

Is that correct?

@mstniy
Copy link
Contributor

mstniy commented Oct 18, 2022

True. In #8231, these two functionalities are atomic with respect to each other. Using the masterKey to set emailVerified along with #s/email changes suppresses email verification, in addition to setting the value of the field.

@mtrezza
Copy link
Member

mtrezza commented Oct 19, 2022

I think we'd want to split this into 2 separate features, it seems each of them have valid use cases on their own, so they shouldn't be depending on each other. Can we address them in 2 separate PRs?

@mstniy
Copy link
Contributor

mstniy commented Oct 19, 2022

The master key can already set the emailVerified field, however, it lacks the ability to bypass email verification for #s/email changes.
Separating these "features" would necessitate the use of some other mechanism to communicate to Parse server that email verification should be bypassed. We think it is intuitive that explicitly setting emailVerified along with a #/email change bypasses verification. What mechanism would you suggest?

@mtrezza
Copy link
Member

mtrezza commented Oct 20, 2022

The context object in the # method. Whatever you add to the context object however must not change the server behavior automatically. It will only be available in Cloud Code to be used in a trigger. So no change in the Parse JSD SDK. The change in Parse Server would be similar to setting the user email verified:

Parse.Cloud.beforeSave(Parse.User, async req => {
  console.log(req.context.sendEmailVerificationEmail); // prints "true" or "false" depending what is passed in `Parse.User.#(context)`
  console.log(req.requireEmailVerification); // prints "true" or "false" depending on the Parse Server option setting

  // Override Parse Server behavior
  req.setEmailVerified = true; // sets the email to verified for the user
  req.setEmailVerified = false; // sets the email to unverified for the user
  req.setSendEmailVerificationEmail = true; // Sends the email verification email
  req.setSendEmailVerificationEmail = false; // Doesn't send the email verification email
});

Maybe setEmailVerified should be set via the master key just like any other property, and only the setSendEmailVerificationEmail needs a special solution.

Such a concept allows for example to set the Parse Server option to not require email verification for users by default. For a certain user (e.g. with suspicious activity) email verification could be required.

@mstniy
Copy link
Contributor

mstniy commented Oct 20, 2022

It will only be available in Cloud Code to be used in a trigger. So no change in the Parse JSD SDK.

I don't see why the master key should not be able to bypass email verification. Sounds like a step backwards to me compared to #8231.

Maybe setEmailVerified should be set via the master key just like any other property, and only the setSendEmailVerificationEmail needs a special solution.

My point exactly. The master key can already set emailVerified.

Such a concept allows for example to set the Parse Server option to not require email verification for users by default. For a certain user (e.g. with suspicious activity) email verification could be required.

That's a good point. The server deciding, during the # process, whether the user should be verified certainly is a different use case. This can be implemented using #8231, but it is somewhat tricky: One would need to disallow direct #s by removing public write permission from the _User CLP and create a cloud function that creates # requests using the master key with the emailVerified field set appropriately, in addition to setting the server to verifyUserEmails: true, preventLoginWithUnverifiedEmail: true.

Or we could modify #8231 such that emailVerified updates are rejected for non-masterkey clients, but is permitted to be set (with its verification-bypassing sideffect) for the master key and any before* trigger. That would cover both use cases while keeping the original Parse # API intact.

@mstniy
Copy link
Contributor

mstniy commented Oct 20, 2022

Some insight on the last point in my previous message for future reference: RestWrite.js runs beforeSave triggers first, and then calls transformUser(), which verifies that either the caller is the master key or the request does not change emailVerified. This would prevent a beforeSave trigger from conditionally bypassing email verification. The order of these operations should be changed such that first emailVerified is checked to disallow clients from setting it, followed by running the beforeSave triggers and finally sending the verification email or not, depending on whether the request sets emailVerified.

@mtrezza
Copy link
Member

mtrezza commented Oct 20, 2022

I don't see why the master key should not be able to bypass email verification.

Yes, I agree that the master key should be able to do that just like with any other non-internal field. Maybe your PR can focus on that part only.

The server deciding, during the # process, whether the user should be verified certainly is a different use case.

As I said, we don't encourage (even though you may have a use case for it), to use the master key on the client side. So any feature suggestion that requires the master key to be used on the client side is considered a "non-feature", or at most it can be a supplemental functionality, but the main feature must be usable without exposing the master key to a client.

Good that you've already studied the code for possible implementations. On a more conceptual level, it seems we are talking about separate functionalities:

  • a) Requiring or omitting email verification on a per-user basis, regardless of Parse Server option
  • b) Manually setting the _User.emailVerified field, just like any other non-internal field
  • c) (Re)-sending or suppressing the verification email, so that a developer can design their own email verification flow

I'm still not entirely sure from #8231 how your process would be, but it may only require (a) and not even (b).

  1. User A signs up with userA@example.com and verifies their email address
  2. User B tries to # with userB@example.com
  3. Sever runs Cloud Code trigger beforeSave(_User) , finds another user already registered with example.com and _User.emailVerified: true (genuinely verified), therefore doesn't send a verification email
  4. User B tries to change email address
  5. Sever runs Cloud Code trigger beforeSave(_User), finds another user already registered with example.com and _User.emailVerified: true (genuinely verified), therefore doesn't send a verification email

The benefit of not using (b) is that you don't "fake-verify" the user's email address, which would pollute your data and there would be no way of telling which verification is genuine. If at a later point you decide to require verification also from that user, you would simply change the trigger logic, without having to deal with polluted data.

@mtrezza mtrezza added bounty:$250 Bounty applies for fixing this issue (Parse Bounty Program) and removed bounty:$100 Bounty applies for fixing this issue (Parse Bounty Program) labels Jan 31, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.3.0-alpha.2

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jun 20, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.4.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Sep 16, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.4.0-alpha.1

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.4.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Nov 16, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bounty:$250 Bounty applies for fixing this issue (Parse Bounty Program) state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature
Projects
None yet
4 participants