fix: original object not retrieved when user logs in with third party auth

[Meglali20]

New Pull Request Checklist

• [x] I am not disclosing a vulnerability.
• [x] I am creating this PR in reference to an issue.

Issue Description

Related issue: issue#7737

Approach

TODOs before merging

• [ ] Add tests
• [ ] Add changes to documentation (guides, repository pages, in-code descriptions)
• [ ] Add security check
• [ ] Add new Parse Error codes to Parse JS SDK
• [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

[parse-github-assistant[bot]]

Thanks for opening this pull request!

• 🎉 We are excited about your hands-on contribution!

[parse-github-assistant[bot]]

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

[Meglali20]

@davimacedo how would you suggest using the same account to log in twice?

[davimacedo]

@Meglali20 I've just checked getMockFacebookProvider and it actually returns same account every time, so you are good with that. It would be good if you could login the user twice and have a success flow calling done() function in the test though.

[Meglali20]

@davimacedo why a success flow on login? that would work but the purpose is to retrieve the original user to check if the user is new or just performing a login. actually, when I ran the test I had no issue and the test demonstrated that it fails and it isn't retrieving the original object on the second login, and the auth data is there.

[davimacedo]

When we write a test for a use-case, we should have a fail path (to check the behavior that we did not expect to happen) and a success path (to check the behavior that we expect to happen). In the way you wrote your test, it will always fail (even if the supposed bug is later on fixed), because there is no call to the done() function. Therefore, we cannot be sure that the test is failing because of a bug that is actually happening or if it is because there is no success path. I'd expect to have a test case that succeeds when Parse.User._logInWith('facebook') is called a single time, and fails when Parse.User._logInWith('facebook') is called twice. Maybe you will have to use getMockFacebookProviderWithIdToken() instead of getMockFacebookProvider () to pass an account that was not used by the other tests. It would be good to also have some message in here done.fail() so we can also easily see that the test is failing when hitting this line.

I've seen you rebases with latest alpha. Is the problem still happening?

[Meglali20]

@davimacedo yes using now version 5.3.0-alpha.4 and the problem is still there.

[mtrezza]

I recently removed many done() from jasmine tests (can't remember which repo), because that was indicated to be a deprecated syntax. Instead of using done(), the test should just return void and not fail. If a fail is expected, use expect.toThrow() or expectAsync.teBeRejectedWith().

[Meglali20]

@mtrezza can you check the test again?

[mtrezza]

@Meglali20 It seems the tests are still failing...

[Meglali20]

@mtrezza what do you mean by failing? Do you mean the test fails to run? because for me the test runs and shows that its fails and if I change it so that it passes I get no issue

[mtrezza]

If you take a look at the tests here, you can see that 16 of them are failing.

[Meglali20]

@mtrezza when I opened the issue and explained it you asked me for a failing test, the test is provided, and when I run npm test spec/ParseUser.spec.js from my side all the test passes except the one that I added fails as intended, also here on GitHub I can see that it fails and that there are some that are still pending. I really didn't get what is the issue, can you please explain it to me? I also just changed _logInWith to logInWith

[mtrezza]

Thanks for simplifying the test, it looks good to me. Do you have a suggestion for a fix?

[Meglali20]

Unfortunately, I have no idea about it since it hasn't been long since I discovered and started using parse so I am not completely familiar with it and didn't look at the code yet. There could be some workaround to avoid this issue but that would be just adding unnecessary extra data to the database. you should be updating the label of the issue so that maybe someone can take a look at it and propose a solution.

[Meglali20]

@mtrezza any update?

[mtrezza]

@parse-community/server anyone has a suggestion for a fix? The failing test here already demos the issue.

[dblythy]

I would assume the issue is happening somewhere around here

https://github.com/parse-community/parse-server/blob/c1e808f9e807fc49508acbde0d8b3f2b901a1638/src/RestWrite.js#L1604

[mtrezza]

@Meglali20 Would you want to rebase this PR on alpha and resolve the conflicts?

[Meglali20]

@mtrezza you mean if I could fix it? I didn't have time yet to look at the parse server code, so I don't think I could fix it at the moment, If you could provide me with some information on how the process works I would maybe try to figure it out.

[mtrezza]

@Meglali20 I didn't mean to fix the issue, just to fix the conflicts in this PR:

There have been commits to alpha since you opened this PR, so there are some conflicts between the latest code and this PR.

[Meglali20]

Resolved conflicts.

[JesperLekland]

Was there ever a fix for this? Or still just a test that showcases the bug?

[Meglali20]

@JesperLekland I am not aware of any fix, I just used a workaround by creating a new class for already registered users.

[parse-github-assistant[bot]]

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