test: Add test for email verification not applying to auth sign-up

[ashish-naik]

Pull Request

This PR is to test Facebook signup error 206 describe in #6511

Closes: #6511

Approach

Added test case for Facebook signup and login.
Calling from iOS Objective-C SDK, saving new PFUser created from Facebook signup fails with error 206.
Was first reported in PS version 5.x.x and observed in v6.2.1 also.

Tasks

[x ] Add tests

[parse-github-assistant]

Thanks for opening this pull request!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.32%. Comparing base (f1469c6) to head (7f73eb3).
Report is 8 commits behind head on alpha.

❗ Current head 7f73eb3 differs from pull request most recent head cf1b227. Consider uploading reports for the commit cf1b227 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #8740      +/-   ##
==========================================
+ Coverage   94.13%   94.32%   +0.18%     
==========================================
  Files         186      186              
  Lines       14687    14773      +86     
==========================================
+ Hits        13826    13934     +108     
+ Misses        861      839      -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mtrezza]

What's the difference between your new test and this existing test?

parse-server/spec/rest.spec.js

Lines 371 to 379 in 5954f0f

	it('test facebook signup and login', done => {
	const data = {
	authData: {
	facebook: {
	id: '8675309',
	access_token: 'jenny',
	},
	},
	};

[ashish-naik]

I guess i was supposed to add test to ParseUser.spec.js.

Can you please check whether this is correct?

[mtrezza]

I think we can using the newer jasmine syntax; I didn't test this, just wrote it as a comment, so it may not be fully correct.
The test still seems to contain some unnecessary code, like

signedUpUser.set('social_id', provider.authData.id);
signedUpUser.set('account_status', 'active');

In any case, the test passes, so we would need to get it to fail. Since you are using the JS SDK methods, there may be a difference to what the Parse iOS SDK sends. To know what the iOS SDK sends to the server you could:

• look at the incoming request on the server side (may be easiest)
• look at the source code of the iOS SDK

Once you know the request that is sent to the server is the same, and you cannot get the test to fail, I suggest you look into your custom Parse Server code as a possible source of the issue.

I ran the CI here, but it failed in all environments, so it's not necessary to run the CI for now. You can execute the test in your local development environment and see whether it fails. Once it fails, it can help to run it on the CI to get more insight.

[ashish-naik]

The test you corrected didn't generate error so i tried to add email adaptor parameters that i have in my index.js.
The test failed with error 206.

Test log is as below

> parse-server@6.3.0-alpha.8 pretest
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=5.3.2} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} mongodb-runner start

  ◠ Starting a MongoDB deployment to test against...
> parse-server@6.3.0-alpha.8 test
> npm run testonly "spec/ParseUser.spec.js"


> parse-server@6.3.0-alpha.8 testonly
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=5.3.2} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} TESTING=1 jasmine "spec/ParseUser.spec.js"

Randomized with seed 60989
Started
Jasmine started
(node:16648) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
warn: DeprecationWarning: The Parse Server option 'allowClientClassCreation' default will change to 'false' in a future version.
warn: DeprecationWarning: The Parse Server option 'allowExpiredAuthDataToken' default will change to 'false' in a future version.
warn: DeprecationWarning: The Parse Server option 'encodeParseObjectInCloudFunction' default will change to 'true' in a future version.
F
  Parse.User testing
    ✗ log in with Facebook and update email for signed up User
      - Unhandled promise rejection: ParseError: 206 Cannot modify user aV67EHAYgx.



Failures:
1) Parse.User testing log in with Facebook and update email for signed up User
  Message:
    Unhandled promise rejection: ParseError: 206 Cannot modify user aV67EHAYgx.
  Stack:
    error properties: Object({ code: 206 })
    Error: Cannot modify user aV67EHAYgx.
        at handleError (/Users/ashishn/Developer/GitHub Projects/Parse-Server-Fix-Facebook-Signup-Error206/node_modules/parse/lib/node/RESTController.js:274:17)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)

Ran 1 of 161 specs
1 spec, 1 failure
Finished in 0.594 seconds
Randomized with seed 60989 (jasmine --random=true --seed=60989)
**************************************************
*                    Failures                    *
**************************************************

1) Parse.User testing log in with Facebook and update email for signed up User
  - Unhandled promise rejection: ParseError: 206 Cannot modify user aV67EHAYgx.

Executed 1 of 161 specs (1 FAILED) (160 SKIPPED) in 0.594 sec.
Randomized with seed 60989.

[mtrezza]

Great, as a next step, remove all custom configuration that is not needed for the test to fail. For example you have resetTokenValidityDuration: 24 * 60 * 60, which likely can be removed, among others. The test still looks too verbose. Are all these lines needed for the test to fail?

user.set('email', 'foo@example.com');
user.set('loginProvider', 'facebook');

[ashish-naik]

Are all these lines needed for the test to fail?

user.set('email', 'foo@example.com');
user.set('loginProvider', 'facebook');

I wanted to replicate my actual code. Removed now.

I was able to identify that combination of these two set to true causes the error. Doesn't fail for any other combination.
have modified the best and test description.

      verifyUserEmails: true,
      preventLoginWithUnverifiedEmail: true,

[mtrezza]

Yes, that's good. We don't see the error logged anymore, but we know what it is anyway. The next step would be to analyze whether this is expected behavior.

[ashish-naik]

I think i also committed the change and created issue in branch. I am not able to pull.
Would be corrected if i reset my commit to position shown by arrow?

[mtrezza]

You can just reset your local branch to the head of the remote branch. Or delete your local branch and pull the branch from the remote.

[mtrezza]

Let's recap: you are signing up a user with Facebook auth, but saving the user afterwards fails if preventLoginWithUnverifiedEmail is set to true, it succeeds if preventLoginWithUnverifiedEmail is set to false.

Error 206 means:

SessionMissing | 206 | A user object without a valid session could not be altered.

So it's possible that a user sign-up is not completed until the email address is verified, even if the user signed up with auth. That could be expected behavior, even though I didn't find anything in the docs. However, it would be logical to assume that if a user is authenticated via a third-party provider, the email verification requirement would be bypassed, since the third-party provider has already done its own verification.

As a next step you could look at the Parse Server code. Since we know it's related to preventLoginWithUnverifiedEmail you could just search for that and set breakpoints. Then run the test and dive deeper into why the test fails.

[mtrezza]

You wrote previously:

I was running server v3.9.0. tried upgrading till 4.0.2 but still failing.

Were you able to determine the exact Parse Server version where this issue started to occur? From your comment above it seems like somewhere between > 3.9.0 and <= 4.0.2. If we know the version, it should be easy to find the change that caused this and we may get a better understanding if this change was intentionally.

[ashish-naik]

I came across this in 3.9.0 but not sure it worked before that or not. I may not have tested signup after upgrading server version.

By the way, it is combination of both verifyUserEmails and preventLoginWithUnverifiedEmail set to true causes the issue.

[mtrezza]

I've corrected the PR title. What's the range of versions we know as of now - what's the latest version where the issue did not occur and the earliest version where the issue did occur? It would be helpful if you could test this out with more versions in between, so we can narrow it down to the exact version where this first started to occur.

[ashish-naik]

i tried to install v3.0.0 to v3.8.0 to 4.2.0 but kept getting errors abt dependencies so couldn't do testing.

I tried to debug more today. Please see if this provides any clue.
Auth.prototype.isUnauthenticated function return false when request is for updating _User after login. The row was created using PFFacebookUtils.logInInBackground(withReadPermissions: facebookPermissions) { called from iOS client.

Variable user in auth is undefined at the time of calling function update(config, auth, className, restWhere, restObject, clientSDK, context) { from Rest
Would this cause of Auth.prototype.isUnauthenticated returning false?

and req.userFromJWT in middleware.js is undefined and sessionToken is blank so Auth seems to be created without user.

 if (req.userFromJWT) {
    req.auth = new _Auth.default.Auth({
      config: req.config,
      installationId: info.installationId,
      isMaster: false,
      user: req.userFromJWT
    });
    return handleRateLimit(req, res, next);
  }

 if (!info.sessionToken) {
    req.auth = new _Auth.default.Auth({
      config: req.config,
      installationId: info.installationId,
      isMaster: false
    });
  }

I debug client code and found that loginWithFacebook returns new PFUser without sessionToken and reason is this in createSessionTokenIfNeeded.
so my update call fails because Auth.prototype.isUnauthenticated returns false when doing database operation.

if (!this.storage.authProvider &&
  // signup call, with
  this.config.preventLoginWithUnverifiedEmail === true &&
  // no login without verification
  this.config.verifyUserEmails) {
    // verification is on
    this.storage.rejectSignup = true;
    return;
  }

Please see this screenshot. Right side popup shows hat current state is to update _User using initialisation parameters post user signup.

[ashish-naik]

Tried to fix the issue.

i updated handleAuthData function with this.storage.authProvider = Object.keys(authData).join(','); placed just before

// No user found with provided authData we need to validate
  if (!results.length) {

This resolved the issue and also ran the test with success.

  Parse.User testing
    ✓ log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true

I found that version 2.6.1 introduced this check.
https://github.com/parse-community/parse-server/blob/2.6.1/src/RestWrite.js#L544

RestWrite.prototype.createSessionTokenIfNeeded = function() {
  if (this.className !== '_User') {
    return;
  }
  if (this.query) {
    return;
  }
  if (!this.storage['authProvider'] // signup call, with
      && this.config.preventLoginWithUnverifiedEmail // no login without verification
      && this.config.verifyUserEmails) { // verification is on
    return; // do not create the session token in that case!
  }
  return this.createSessionToken();
}

In latest version this check is bit different

if (
    !this.storage.authProvider && // signup call, with
    this.config.preventLoginWithUnverifiedEmail === true && // no login without verification
    this.config.verifyUserEmails
  ) {
    // verification is on
    this.storage.rejectSignup = true;
    return;
  }

[ashish-naik]

Previous discussions on this issue

#4155

#4142

[ashish-naik]

@mtrezza i have again tried to reproduce with latest version and it is still failing. Below is the test log.

This solution still works.

In meantime, i have added a workaround in beforeLogin trigger to handle this issue.

Parse.Cloud.beforeLogin(async request => {
  const { object: user }  = request;
  if (user.get('login_provider') == "email" && user.get('emailVerified') != true) {
    logger.error(`beforeLogin: User ${user.id} is not verified`)
    throw new Parse.Error(302,'Email is unverified')
   }
});

Test result

% npm test spec/ParseUser.spec.js

> parse-server@7.1.0-alpha.6 pretest
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=5.3.2} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} mongodb-runner start -t ${MONGODB_TOPOLOGY} --version ${MONGODB_VERSION} -- --port 27017

Server started and running at mongodb://127.0.0.1:27017/
Run the following command to stop the instance:
mongodb-runner stop --id=3c53f8e2-946a-4012-a356-ce497958e619

> parse-server@7.1.0-alpha.6 test
> npm run testonly "spec/ParseUser.spec.js"


> parse-server@7.1.0-alpha.6 testonly
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=5.3.2} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} TESTING=1 jasmine "spec/ParseUser.spec.js"

Randomized with seed 34178
Started
Jasmine started
warn: DeprecationWarning: The Parse Server option 'encodeParseObjectInCloudFunction' default will change to 'true' in a future version.
.
  allowExpiredAuthDataToken option
    ✓ should default false
.    ✓ should enforce boolean values
.    ✓ should accept true value
.    ✓ should accept false value

.  Parse.User testing
    ✓ user login with non-string username with REST API
.    ✓ user login with context
.    ✓ user login wrong password
.    ✓ login with provider should be blockable by beforeLogin
.    ✓ user with missing username
.    ✓ login with provider should not call beforeSave trigger
.    ✓ user login wrong username
.    ✓ should allow PUT request with stale auth Data
.    ✓ saving user after browser refresh
.    ✓ become
.    ✓ signup with provider should not call beforeLogin trigger
.    ✓ user sign up instance method
.    ✓ querying for users only gets the expected fields
.    ✓ should fail linking with existing
.    ✓ user save should fail with invalid email
.    ✓ log in with provider failed
.    ✓ link multiple providers
.    ✓ unset user email
.    ✓ link with provider
.    ✓ cannot delete session if no sessionToken
.    ✓ should not fail querying non existing relations
.    ✓ user sign up class method
.    ✓ should be able to update user with authData passed
.    ✓ log in with provider cancelled
.    ✓ can login when both email and username are passed
.    ✓ should not allow login with expired authData token since allowExpiredAuthDataToken is set to false by default
.    ✓ should not retrieve hidden fields on GET users/me (#3432)
.    ✓ password format matches hosted parse
.    ✓ should not revoke session tokens if the server is configures to not revoke session tokens
.    ✓ logout with provider should call afterLogout trigger
.    ✓ should throw OBJECT_NOT_FOUND instead of SESSION_MISSING when using masterKey
.    ✓ fails to login when email and username are not provided
.    ✓ can not set authdata to null
.    ✓ user cannot update email to existing user
.    ✓ link with provider for already linked user
.    ✓ should revoke sessions when converting anonymous user to "normal" user
.    ✓ user modified while saving with unsaved child
.    ✓ user associations
.    ✓ should revoke sessions when setting paswword with masterKey (#3289)
.    ✓ should not create extraneous session tokens
.    ✓ should let legacy users without ACL login
.    ✓ fails to login when email doesn't match username
.    ✓ should not overwrite username when unlinking facebook user (regression test for #1532)
.    ✓ link with provider failed
.    ✓ can login with email
.    ✓ should return current session with expired expiration date
.    ✓ link with provider via sessionToken should not create new sessionToken (Regression #5799)
.    ✓ become sends token back
.    ✓ allows login when providing email as username
.    ✓ log in with explicit facebook auth data
.    ✓ should fail linking with existing through REST
.    ✓ should not retrieve hidden fields on GET users/id (#3432)
.    ✓ login with provider should be blockable by beforeLogin even when the user has a attached file
.    ✓ fails to login when password is not provided
.    ✓ should send email when upgrading from anon
.    ✓ should allow updates to fields with maintenanceKey
.    ✓ create session from user
.    ✓ authenticated check
.    ✓ user queries
.    ✓ user loaded from localStorage from signup
.    ✓ user login with files
.    ✓ log in with provider and update token
.    ✓ should not allow updates to emailVerified
.    ✓ link async with explicit facebook auth data
.    ✓ cannot saveAll with non-authed user
.    ✓ should properly error when password is missing
    ✗ log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true
      - Expected [object Promise] to be resolved.
.    ✓ should be let masterKey lock user out with authData
.    ✓ user updates
.    ✓ handles properly when 2 users share username / email pairs, counterpart
.    ✓ user modified while saving
.    ✓ should not retrieve hidden fields on login (#3432)
.    ✓ async methods
.    ✓ unlink with provider
.    ✓ log in with provider
.    ✓ user.isCurrent
.    ✓ user get session from token on signup
.    ✓ should have authData in beforeSave and afterSave
.    ✓ fails to login when username doesn't match email
.    ✓ should cleanup null authData keys (regression test for #935)
.    ✓ user loaded from localStorage from login
.
    issue #4897
      ✓ should be able to login with a legacy user (no ACL)

.    ✓ contained in user array queries
.    ✓ user get session from token on anonymous login
.    ✓ ignore setting authdata to undefined
.    ✓ login with provider should call beforeLogin trigger
.    ✓ should let masterKey lockout user
.    ✓ link with provider cancelled
.    ✓ never locks himself up
.    ✓ cannot save non-authed user
.    ✓ set password then change password
.    ✓ should aftersave with full object
.    ✓ link with provider should return sessionToken
.    ✓ Invalid session tokens are rejected
.    ✓ user signup class method uses subclassing
.    ✓ user with missing password
.    ✓ unlink and link
.    ✓ incorrect login with provider should not call beforeLogin trigger
.    ✓ should not call beforeLogin with become
.    ✓ count users
.    ✓ should cleanup null authData keys ParseUser update (regression test for #1198, #2252)
.    ✓ should respect ACL without locking user out
.    ✓ user authData should be available in cloudcode (#2342)
.    ✓ log in with provider with files
.
    case insensitive signup not allowed
      ✓ signup should fail with duplicate case insensitive username with field specific setter
.      ✓ edit should fail with duplicate case insensitive email
.      ✓ signup should fail with duplicate case insensitive username with basic setter
.      ✓ signup should fail with duplicate case insensitive email
.
      anonymous users
        ✓ should not fail on case insensitive matches

.    ✓ user stupid subclassing
.    ✓ user get session from token on username/password login
.    ✓ should not allow updates to hidden fields
.    ✓ user login
.    ✓ handles properly when 2 users share username / email pairs
.    ✓ should not serve null authData keys
.    ✓ cannot delete non-authed user
.    ✓ log in with provider twice
.    ✓ user login using POST with REST API
.    ✓ saving a user signs them up but doesn't log them in
.    ✓ link multiple providers and updates token
.    ✓ retrieve user data from fetch, make sure the session token hasn't changed
.    ✓ current user
.    ✓ current user isn't dirty
.    ✓ returns authData when authed and logged in with provider (regression test for #1498)
.    ✓ changes to a user should update the cache
.    ✓ ensure logout works
.    ✓ querying for users doesn't get session tokens
.    ✓ log in async with explicit facebook auth data
.    ✓ can login with email through query string
.    ✓ user sign up with container class
.    ✓ user login with non-string username with REST API (again)
.    ✓ changing password clears sessions
.    ✓ user signup should error if email taken
.    ✓ user update session with other field
.    ✓ link multiple providers and update token
.    ✓ only creates a single session for an installation / user pair (#2885)
.    ✓ user on disk gets updated after save
.    ✓ should strip out authdata in LiveQuery
.    ✓ cannot login with email and invalid password
*.    ✓ link with explicit facebook auth data
.    ✓ cannot update session if invalid or no session token
.    ✓ should not send email when email is not a string
.    ✓ session expiresAt correct format
.    ✓ does not duplicate session when logging in multiple times #3451
.    ✓ user login with enforcePrivateUsers
.    ✓ get session only for current user
.    ✓ should fail to become user with expired token
.    ✓ support user/password signup with empty authData block
.    ✓ test parse user become
.    ✓ should throw when enforcePrivateUsers is invalid
.    ✓ delete session by object

.  Security Advisory GHSA-8w3j-g983-8jh5
    ✓ should ignore authData field
.    ✓ should validate credentials first and check if account already linked afterwards ()

.  login as other user
    ✓ rejects creating a session for another user with invalid parameters
.    ✓ rejects creating a session for another user without the master key
.    ✓ rejects creating a session for another user if the user does not exist
.    ✓ allows creating a session for another user with the master key

.  allowClientClassCreation option
    ✓ should accept false value
.    ✓ should default false
.    ✓ should enforce boolean values
.    ✓ should accept true value



Failures:
1) Parse.User testing log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true
  Message:
    Expected [object Promise] to be resolved.
  Stack:
    Error
        at <Jasmine>
        at UserContext.<anonymous> (/Users/ashishn/Developer/GitHub Projects/Parse-Server-Fix-Facebook-Signup-Error206/spec/ParseUser.spec.js:1290:36)
        at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
Pending:

1) Parse.User testing should not send a verification email if the user signed up using oauth
  this test fails.  See: https://github.com/parse-community/parse-server/issues/5097

166 specs, 1 failure, 1 pending spec
Finished in 20.468 seconds
Randomized with seed 34178 (jasmine --random=true --seed=34178)
**************************************************
*                    Failures                    *
**************************************************

1) Parse.User testing log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true
  - Expected [object Promise] to be resolved.

**************************************************
*                    Pending                     *
**************************************************

1) Parse.User testing should not send a verification email if the user signed up using oauth
  this test fails.  See: https://github.com/parse-community/parse-server/issues/5097

Executed 165 of 166 specs (1 FAILED) (1 PENDING) in 20 secs.
Randomized with seed 34178.

I feel i have made some mistake merging my branch.

[parse-github-assistant]

I will reformat the title to use the proper commit message syntax.

[mtrezza]

Now that we have a test case, it should be easy to debug and find out want's going on inside Parse Server.