parse-community / parse-server

Parse Server for Node.js / Express
https://parseplatform.org
Apache License 2.0
20.71k stars 4.76k forks source link

AuthenticationProviders should only create a single user with REST API #6385

Closed yomybaby closed 4 years ago

yomybaby commented 4 years ago

Issue Description

AuthenticationProviders should only create a single user with REST API. AuthenticationAdapters.spec.js has a test case for this.

  it('should only create a single user with REST API', done => {
    let objectId;
    createOAuthUser((error, response, body) => {
      expect(error).toBe(null);
      const b = body;
      expect(b.objectId).not.toBeNull();
      expect(b.objectId).not.toBeUndefined();
      objectId = b.objectId;

      createOAuthUser((error, response, body) => {
        expect(error).toBe(null);
        const b = body;
        expect(b.objectId).not.toBeNull();
        expect(b.objectId).not.toBeUndefined();
        expect(b.objectId).toBe(objectId);
        done();
      });
    });
  });

This test case try to make two POST request with the same authData in a serial way. In a parallel way, it can make double users.

Steps to reproduce

I wrote a test case for this.

it('should only create a single user with REST API (simultaneous requests)', async () => {
    const results = await Promise.all([
      createOAuthUser(),
      createOAuthUser(),
      createOAuthUser(),
    ]);
    expect(results[0].body.objectId).toBe(results[1].body.objectId);
    expect(results[1].body.objectId).toBe(results[2].body.objectId);
  });

Please add this case to AuthenticationAdapters.spec.js and run test.

Expected Results

Test pass.

Actual Outcome

Test fail

example output

Failures:
1) AuthenticationProviders should only create a single user with REST API (simultaneous requests)
  Message:
    Expected 'qMqAZfxQsd' to be 'bl6vjyiUBG'.
  Stack:
    Error: Expected 'qMqAZfxQsd' to be 'bl6vjyiUBG'.
        at <Jasmine>
        at UserContext.it (/Users/codejong/Documents/_clone/parse-server/spec/AuthenticationAdapters.spec.js:248:38)
        at process._tickCallback (internal/process/next_tick.js:68:7)
  Message:
    Expected 'bl6vjyiUBG' to be 'vD9ZRiRmJp'.
  Stack:
    Error: Expected 'bl6vjyiUBG' to be 'vD9ZRiRmJp'.
        at <Jasmine>
        at UserContext.it (/Users/codejong/Documents/_clone/parse-server/spec/AuthenticationAdapters.spec.js:249:38)
        at process._tickCallback (internal/process/next_tick.js:68:7)

Environment Setup

dplewis commented 4 years ago

This look like expected behavior to me. Create0AuthUser just creates a user.

Lets say I have to two users creating accounts at the same time. Are they two separate users or same user?

yomybaby commented 4 years ago

I think that this is a bug. With the same auth data, it have to create only one user.

Lets say I have to two users creating accounts using same SNS account at the same time. Are they two separate users or same user?

This answer is "same user", right? Because they are using same SNS account.

Create0AuthUser use same auth data(id) every times. (link)

For example, If you try to signup/login with the same facebook account many times,

Existing test code test it only in a serial way not In a parallel way.

JeffGuKang commented 4 years ago

@dplewis I got a pain with same issue. 🐛

Sometimes multiple same accounts(having same authData) are created by multiple request from the one user when I use linkWith() for social login. And unique check is not work through username because username of _User is randomly generated.

This situation happens by poor connection, slow DB, request by fast double click and so on. In my case, I got a bunch of duplicated users when my db server was slow. And they threw one star to my application 😭

I suggest the option for auth options to choose username from authData. or integrate username generation to adapter.

For example,

Apple authData's form is apple: {id: 'xxxx', token: 'aaa'}. And we want to generate unique username from apple.id

new ParseServer({
...
auth: {
    kakao: kakaoAuthAdapter,
    apple: {
      client_id: process.env.APPLE_SIGNIN_CLIENT_ID,
      username: 'apple.id'
    },
  },
...
})

Or in adapter,

  _createClass(Apple, [{
    key: "uniqueUsername",
    value: function validateAuthData(authData, options) {
        return authData.apple.id
    }

...