User.linkWith doesn't remove anonymous auth

[swittk]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server.

Issue Description

Calling user.linkWith() server-side doesn't remove the "anonymous" field from the authData object field, causing Parse.AnonymousUtils to still report the user as an Anonymous user.

Steps to reproduce

Parse.Cloud.define('linkWithAccount', async (request) => {
  const user = request.user;
  if (!user) throw 'NoUser';
  const provider = request.params.provider as string;
  const authData = request.params.authData as any;
  if (!provider) throw 'NoProvider';
  else if (!authData) throw 'NoAuthData';

  await user.linkWith(provider, { authData }, {
    useMasterKey: true,
    sessionToken: user.getSessionToken()
  });
  
  return { ok: true };
});

Actual Outcome

The authData field on Parse Database still has the "anonymous" field, resulting in Parse.AnonymousUtils to still report the user as an Anonymous user

{
  "anonymous": {
    "id": "<random-string>"
  },
  "<linked_service_name>": {
    ...linked_service_data
  }
}

Expected Outcome

Inpecting the authData field on Parse Database not have the anonymous field (should only have..)

{
  "<linked_service_name>": {
    ...linked_service_data
  }
}

Failing Test Case / Pull Request

• 🤩 I submitted a PR with a fix and a test case.
• 🧐 I submitted a PR with a failing test case.

Environment

Server

• Parse Server version: 4.5.0
• Operating system: Mac OS X Mojave 10.14.6
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Heroku

Database

• System (MongoDB or Postgres): MongoDB
• Database version: FILL_THIS_OUT
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): MongoDB Atlas

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): Javascript (React-Native)
• SDK version: 2.19.0

Logs

[mtrezza]

Thanks for reporting!

Would you want to submit a PR with a failing test case for this, so we can verify the issue?

[cbaker6]

Currently, the server isn't designed to handle this and the Client SDKs are expected to strip anonymous when linking:

Objective-C SDK: https://github.com/parse-community/Parse-SDK-iOS-OSX/blob/2e4242c683e645a7d78ff37dd34398119178c0c5/Parse/Parse/PFUser.m#L862-L884

Parse-Swift SDK: https://github.com/parse-community/Parse-Swift/blob/1636285beb51d9bf33230d5f80a9db5f8ec8707c/Sources/ParseSwift/Authentication/Protocols/ParseAuthentication.swift#L432-L486

Android SDK: https://github.com/parse-community/Parse-SDK-Android/blob/ea2a6a7d7b761a568dfb261cd0694699f3c5a978/parse/src/main/java/com/parse/ParseUser.java#L1212-L1222

I think React-Native and Cloud Code depends on the JS SDK (I don't use React-Native, so I'm guessing). If so, it looks like the JS SDK is where the PR is needed as I currently only see it stripping anonymous when the username field is changed. It should do something similar to the name striping when linking occurs (you can use one of the aforementioned SDKs as a reference):

JS SDK:

Parse-SDK-JS/src/ParseUser.js

Lines 338 to 346 in a1995ba

	setUsername(username: string) {
	// Strip anonymity
	const authData = this.get('authData');
	if (authData && typeof authData === 'object' && authData.hasOwnProperty('anonymous')) {
	// We need to set anonymous to null instead of deleting it in order to remove it from Parse.
	authData.anonymous = null;
	}
	this.set('username', username);
	}

[swittk]

React Native does use the JS SDK, but I'm referring to the Cloud Code in general in this case (since it is the same for whatever client-side is).

I did observe the behavior of setUsername(...) clearing the anonymous field before, so I've been using a setUsername() call right after linking authData for a while as a sort of hack to clear the anonymous field, and that has been functional and stable so far 😅. I just pointed this issue out since it seems unintuitive.

I was inspecting the cloud code repository before, but didn't know it was based on the JS SDK. That is new knowledge to me.

As for the failing test case.. I'm not sure where to write it.. Also I don't think providing my own authToken would be a good idea.
However, I can assure that this occurs with all providers I've tested, whether it be Apple, Facebook, or Google. Simply sending the appropriate token from the client side to be linked on the server side would cause this behavior. Which is probably due to the fact that @cbaker6 mentioned that anonymous user stripping only occurs when setUsername() is called.

I don't know if this helps the test case thing though

// Client side: Example with apple authentication
async function AppleSignInWithCredentials(creds: AppleAuthenticationCredential) {
  const authData = { id: creds.user, token: creds.identityToken };
  await Parse.Cloud.run('linkWithAccount', { provider: 'apple', authData });
}

// Server side: The linking function
Parse.Cloud.define('linkWithAccount', async (request) => {
  const user = request.user;
  if (!user) throw 'NoUser';
  const provider = request.params.provider as string;
  const authData = request.params.authData as any;
  if (!provider) { throw 'NoProvider'; } else if (!authData) { throw 'NoAuthData'; }
  await user.linkWith(provider, { authData }, {
    useMasterKey: true,
    sessionToken: user.getSessionToken()
  });
  /** This block of code solves the problem
   * user.setUsername(user.getUsername() || user.id);
   * await user.save(null, { useMasterKey: true });
   */
  return { ok: true };
});

[mtrezza]

The change would then be necessary in the JS SDK.

I don't think providing my own authToken would be a good idea.

You don't need a real token but can just mock the server response in the test. Or you can use an integration test which spins up a server.

There are 2 tests necessary. Expect that auth field is stripped before calling linkWith and:

• a) not present afterwards if linkWith succeeded
• b) restored afterwards if linkWith failed

I will transfer this issue to the JS SDK for the PR to be made there.

Thanks @cbaker6 for the in-depth analysis.