-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
test: Add test for email verification not applying to auth sign-up #8740
base: alpha
Are you sure you want to change the base?
test: Add test for email verification not applying to auth sign-up #8740
Conversation
…p to test error 206 raised in parse-community#6511
Thanks for opening this pull request! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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', | |
}, | |
}, | |
}; |
I guess i was supposed to add test to ParseUser.spec.js. Can you please check whether this is correct? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Co-authored-by: Manuel <[email protected]> Signed-off-by: Ashish Naik <[email protected]>
… This causes test to fail with error 206.
The test you corrected didn't generate error so i tried to add email adaptor parameters that i have in my index.js. Test log is as below
|
Great, as a next step, remove all custom configuration that is not needed for the test to fail. For example you have
|
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.
|
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. |
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
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. |
Let's recap: you are signing up a user with Facebook auth, but saving the user afterwards fails if Error 206 means:
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 |
…e.authProvider early to test.
…e.authProvider early to test
Tried to fix the issue. i updated handleAuthData function with
This resolved the issue and also ran the test with success.
I found that version 2.6.1 introduced this check.
In latest version this check is bit different
|
* commit 'b1e1bf6708f5d32b2846e66de40f48fb0ec1dc86': chore(release): 6.4.0-beta.1 [skip ci] release chore(release): 6.3.0 [skip ci] release chore(release): 6.3.0-alpha.9 [skip ci] perf: Improve performance of recursive pointer iterations (parse-community#8741) refactor: Parse Pointer allows to access internal Parse Server classes and circumvent `beforeFind` query trigger (parse-community#8734) chore(release): 6.2.2 [skip ci] fix: Parse Pointer allows to access internal Parse Server classes and circumvent `beforeFind` query trigger; fixes security vulnerability [GHSA-fcv6-fg5r-jm9q](GHSA-fcv6-fg5r-jm9q) refactor: Remote code execution via MongoDB BSON parser through prototype pollution; fixes security vulnerability [GHSA-462x-c3jw-7vr6](GHSA-462x-c3jw-7vr6) (parse-community#8677) chore(release): 6.2.1 [skip ci] fix: Remote code execution via MongoDB BSON parser through prototype pollution; fixes security vulnerability [GHSA-462x-c3jw-7vr6](GHSA-462x-c3jw-7vr6) (parse-community#8674) refactor: Add option to convert `Parse.Object` to instance in Cloud Function payload (parse-community#8656)
…B-signup-error206 into Facebook-signup-error206
Reproducing error using test it('log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true' Under ParseUser.spec.js. Signed-off-by: Ashish Naik <[email protected]>
@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.
Test result
I feel i have made some mistake merging my branch. |
I will reformat the title to use the proper commit message syntax. |
Signed-off-by: Manuel <[email protected]>
Now that we have a test case, it should be easy to debug and find out want's going on inside Parse Server. |
Hi, i tried debugging but could not reproduce. Test however still fails.
Client close calling the cloud code is as below.
latest test log
|
If the test fails as expected, could you just debug to test to find out why it fails? That should give you a hint on how to fix the bug. |
Added test "'link and login with provider with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true'" under ParseUser.spec.js. Signed-off-by: Ashish Naik <[email protected]>
I don't know how to debug a test. I have to run using jest i guess but couldn't figure it out. but i realised the test i had written seems incorrect. I tries to save user after linking. I believe session token doesn't get created on linking, it is created on successful login. So i added this test that links and then logs in. When I make preventLoginWithUnverifiedEmail= false, it times out even after 10 minutes at expectAsync step.
I faced these issues when i am running tests. |
This is how you would sign up using an auth adapter and then get the session token:
If you search existing tests for Test debugging:
|
Signed-off-by: Ashish Naik <[email protected]>
Rewrote test "'log in with Facebook and save signed up User with verifyUserEmails=true and preventLoginWithUnverifiedEmail=true'" . Signed-off-by: Ashish Naik <[email protected]>
Thanks for those hints. I was able to debug. The issue start in handleAuthData where if users with provided authData is not found then function returns from this condition.
next is In The resolution is placing Hope i was able to explain. I have made change in RestWrite.js and ParseUser.spec.js. |
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