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.

Sign up for GitHub

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

Jump to bottom

preventLoginWithUnverifiedEmail does not prevent auto login after signup #4142

Closed
tolgaatam opened this issue Sep 10, 2017 · 7 comments
Closed

preventLoginWithUnverifiedEmail does not prevent auto login after signup #4142

tolgaatam opened this issue Sep 10, 2017 · 7 comments
Assignees
@flovilmart
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@tolgaatam
Copy link

tolgaatam commented Sep 10, 2017 •
edited
Loading

Issue Description

preventLoginWithUnverifiedEmail and verifyUserEmails flags set to true together do not prevent auto login after signup which bypasses the security measures

Steps to reproduce

Create a parse server application on local/remote server. set options preventLoginWithUnverifiedEmail and verifyUserEmails options to true and supply a working email adapter.
Create a test client and send a valid signup request. Check sessions to see if a session is created for the user

Expected Results

Expected is that the user is not ever allowed to login before verifying the email. Hence, a session should not be created for the user until emailVerified field of the user is set true.

Actual Outcome

For the first time, the user is logged in automatically to the system after signup. Consecutive manual logins fail with an error message indicating the lack of email verification, as expected.

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.6.0
    • Operating System: Amazon Linux x86
    • Hardware: t2.small test server, 1vCPU 2GB ram
    • Localhost or remote server? : AWS Elastic Beanstalk

  • Database

    • MongoDB version: 3.4.7
    • Storage engine: WiredTiger
    • Hardware: test database server with shared ram 512mb db storage
    • Localhost or remote server? : MongoDB Atlas

Logs/Trace
@flovilmart flovilmart added the type:bug Impaired feature or lacking behavior that is likely assumed label Sep 10, 2017
@flovilmart flovilmart self-assigned this Sep 10, 2017
@flovilmart flovilmart added pr-submitted type:bug Impaired feature or lacking behavior that is likely assumed and removed type:bug Impaired feature or lacking behavior that is likely assumed labels Sep 10, 2017
flovilmart added a commit that referenced this issue Sep 11, 2017
@flovilmart
fix: Issue #4142 (#4144)
a660a0c 
* Tweaks test in order to show the error

- Session is effectively created when it should not

* Do not create a session when users need verified accounts on signup
@tolgaatam
Copy link
Author

with the fix that is landed, the user is still returned to the client without creating a session token. to create the behavior that we want, we still need to log the user out on the client-side after signup to clean the cache regarding user information.

@flovilmart
Copy link
Contributor

You don’t need to log the user out, actually, your signup call succeeds as a user is effectively created in the database, what do you expect?
Your current user isn’t able to have any authenticated call.

Can you provide an example of the usage you’d like to have?

@natanrolnik
Copy link
Contributor

@flovilmart remember what we discussed? I think this is exactly the case I told you.

@tolgaatam instead of checking for a current user, the client needs to check if the current user has a sessionToken.

@flovilmart
Copy link
Contributor

Yep, but there’s no way to make the call looks like the signup is OK, (because it is) and that an authenticated user is not the current one without rewriting all SDK’s. If one tries to save the current user, this will fail, because there’s no session token, so fall back to a login/ signup flow.

I don’t believe we should change anything in the SDK’s, as there’s an obvious way to check he state.

@natanrolnik
Copy link
Contributor

@flovilmart totally agree with you here.

Therefore, we need to document the need to check for sessionToken instead of currentUser whenever this setting is configured in the server.

@flovilmart
Copy link
Contributor

Yep!

@natanrolnik natanrolnik mentioned this issue Sep 13, 2017
Closed
@flovilmart
Copy link
Contributor

Closing as PR is merged.

@snyk-bot snyk-bot mentioned this issue May 21, 2021
Merged
@ashish-naik ashish-naik mentioned this issue Sep 17, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@flovilmart flovilmart

Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@flovilmart @natanrolnik @tolgaatam