Skip to content

Commit 853bfe1

Browse files
mtrezzamtrezza
authored
fix: Concurrent signup with same authentication creates duplicate users (#10149)
1 parent 6d84496 commit 853bfe1

11 files changed

Lines changed: 450 additions & 20 deletions

File tree

‎spec/AuthDataUniqueIndex.spec.js‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
'use strict';
2+
3+
const request = require('../lib/request');
4+
const Config = require('../lib/Config');
5+
6+
describe('AuthData Unique Index', () => {
7+
const fakeAuthProvider = {
8+
validateAppId: () => Promise.resolve(),
9+
validateAuthData: () => Promise.resolve(),
10+
};
11+
12+
beforeEach(async () => {
13+
await reconfigureServer({ auth: { fakeAuthProvider } });
14+
});
15+
16+
it('should prevent concurrent signups with the same authData from creating duplicate users', async () => {
17+
const authData = { fakeAuthProvider: { id: 'duplicate-test-id', token: 'token1' } };
18+
19+
// Fire multiple concurrent signup requests with the same authData
20+
const concurrentRequests = Array.from({ length: 5 }, () =>
21+
request({
22+
method: 'POST',
23+
headers: {
24+
'X-Parse-Application-Id': 'test',
25+
'X-Parse-REST-API-Key': 'rest',
26+
'Content-Type': 'application/json',
27+
},
28+
url: 'http://localhost:8378/1/users',
29+
body: { authData },
30+
}).then(
31+
response => ({ success: true, data: response.data }),
32+
error => ({ success: false, error: error.data || error.message })
33+
)
34+
);
35+
36+
const results = await Promise.all(concurrentRequests);
37+
const successes = results.filter(r => r.success);
38+
const failures = results.filter(r => !r.success);
39+
40+
// All should either succeed (returning the same user) or fail with "this auth is already used"
41+
// The key invariant: only ONE unique objectId should exist
42+
const uniqueObjectIds = new Set(successes.map(r => r.data.objectId));
43+
expect(uniqueObjectIds.size).toBe(1);
44+
45+
// Failures should be "this auth is already used" errors
46+
for (const failure of failures) {
47+
expect(failure.error.code).toBe(208);
48+
expect(failure.error.error).toBe('this auth is already used');
49+
}
50+
51+
// Verify only one user exists in the database with this authData
52+
const query = new Parse.Query('_User');
53+
query.equalTo('authData.fakeAuthProvider.id', 'duplicate-test-id');
54+
const users = await query.find({ useMasterKey: true });
55+
expect(users.length).toBe(1);
56+
});
57+
58+
it('should prevent concurrent signups via batch endpoint with same authData', async () => {
59+
const authData = { fakeAuthProvider: { id: 'batch-race-test-id', token: 'token1' } };
60+
61+
const response = await request({
62+
method: 'POST',
63+
headers: {
64+
'X-Parse-Application-Id': 'test',
65+
'X-Parse-REST-API-Key': 'rest',
66+
'Content-Type': 'application/json',
67+
},
68+
url: 'http://localhost:8378/1/batch',
69+
body: {
70+
requests: Array.from({ length: 3 }, () => ({
71+
method: 'POST',
72+
path: '/1/users',
73+
body: { authData },
74+
})),
75+
},
76+
});
77+
78+
const results = response.data;
79+
const successes = results.filter(r => r.success);
80+
const failures = results.filter(r => r.error);
81+
82+
// All successes should reference the same user
83+
const uniqueObjectIds = new Set(successes.map(r => r.success.objectId));
84+
expect(uniqueObjectIds.size).toBe(1);
85+
86+
// Failures should be "this auth is already used" errors
87+
for (const failure of failures) {
88+
expect(failure.error.code).toBe(208);
89+
expect(failure.error.error).toBe('this auth is already used');
90+
}
91+
92+
// Verify only one user exists in the database with this authData
93+
const query = new Parse.Query('_User');
94+
query.equalTo('authData.fakeAuthProvider.id', 'batch-race-test-id');
95+
const users = await query.find({ useMasterKey: true });
96+
expect(users.length).toBe(1);
97+
});
98+
99+
it('should allow sequential signups with different authData IDs', async () => {
100+
const user1 = await Parse.User.logInWith('fakeAuthProvider', {
101+
authData: { id: 'user-id-1', token: 'token1' },
102+
});
103+
const user2 = await Parse.User.logInWith('fakeAuthProvider', {
104+
authData: { id: 'user-id-2', token: 'token2' },
105+
});
106+
107+
expect(user1.id).toBeDefined();
108+
expect(user2.id).toBeDefined();
109+
expect(user1.id).not.toBe(user2.id);
110+
});
111+
112+
it('should still allow login with authData after successful signup', async () => {
113+
const authPayload = { authData: { id: 'login-test-id', token: 'token1' } };
114+
115+
// Signup
116+
const user1 = await Parse.User.logInWith('fakeAuthProvider', authPayload);
117+
expect(user1.id).toBeDefined();
118+
119+
// Login again with same authData — should return same user
120+
const user2 = await Parse.User.logInWith('fakeAuthProvider', authPayload);
121+
expect(user2.id).toBe(user1.id);
122+
});
123+
124+
it('should skip startup index creation when createIndexAuthDataUniqueness is false', async () => {
125+
const config = Config.get('test');
126+
const adapter = config.database.adapter;
127+
const spy = spyOn(adapter, 'ensureAuthDataUniqueness').and.callThrough();
128+
129+
// Temporarily set the option to false
130+
const originalOptions = config.database.options.databaseOptions;
131+
config.database.options.databaseOptions = { createIndexAuthDataUniqueness: false };
132+
133+
await config.database.performInitialization();
134+
expect(spy).not.toHaveBeenCalled();
135+
136+
// Restore original options
137+
config.database.options.databaseOptions = originalOptions;
138+
});
139+
140+
it('should handle calling ensureAuthDataUniqueness multiple times (idempotent)', async () => {
141+
const config = Config.get('test');
142+
const adapter = config.database.adapter;
143+
144+
// Both calls should succeed (index creation is idempotent)
145+
await adapter.ensureAuthDataUniqueness('fakeAuthProvider');
146+
await adapter.ensureAuthDataUniqueness('fakeAuthProvider');
147+
});
148+
149+
it('should log warning when index creation fails due to existing duplicates', async () => {
150+
const config = Config.get('test');
151+
const adapter = config.database.adapter;
152+
153+
// Spy on the adapter to simulate a duplicate value error
154+
spyOn(adapter, 'ensureAuthDataUniqueness').and.callFake(() => {
155+
return Promise.reject(
156+
new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'duplicates exist')
157+
);
158+
});
159+
160+
const logSpy = spyOn(require('../lib/logger').logger, 'warn');
161+
162+
// Re-run performInitialization — should warn but not throw
163+
await config.database.performInitialization();
164+
expect(logSpy).toHaveBeenCalledWith(
165+
jasmine.stringContaining('Unable to ensure uniqueness for auth data provider'),
166+
jasmine.anything()
167+
);
168+
});
169+
170+
it('should prevent concurrent signups with same anonymous authData', async () => {
171+
const anonymousId = 'anon-race-test-id';
172+
const authData = { anonymous: { id: anonymousId } };
173+
174+
const concurrentRequests = Array.from({ length: 5 }, () =>
175+
request({
176+
method: 'POST',
177+
headers: {
178+
'X-Parse-Application-Id': 'test',
179+
'X-Parse-REST-API-Key': 'rest',
180+
'Content-Type': 'application/json',
181+
},
182+
url: 'http://localhost:8378/1/users',
183+
body: { authData },
184+
}).then(
185+
response => ({ success: true, data: response.data }),
186+
error => ({ success: false, error: error.data || error.message })
187+
)
188+
);
189+
190+
const results = await Promise.all(concurrentRequests);
191+
const successes = results.filter(r => r.success);
192+
const failures = results.filter(r => !r.success);
193+
194+
// All successes should reference the same user
195+
const uniqueObjectIds = new Set(successes.map(r => r.data.objectId));
196+
expect(uniqueObjectIds.size).toBe(1);
197+
198+
// Failures should be "this auth is already used" errors
199+
for (const failure of failures) {
200+
expect(failure.error.code).toBe(208);
201+
expect(failure.error.error).toBe('this auth is already used');
202+
}
203+
204+
// Verify only one user exists in the database with this authData
205+
const query = new Parse.Query('_User');
206+
query.equalTo('authData.anonymous.id', anonymousId);
207+
const users = await query.find({ useMasterKey: true });
208+
expect(users.length).toBe(1);
209+
});
210+
});

‎spec/DatabaseController.spec.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,11 @@ describe('DatabaseController', function () {
415415
email_1: { email: 1 },
416416
_email_verify_token: { _email_verify_token: 1 },
417417
_perishable_token: { _perishable_token: 1 },
418+
_auth_data_custom_id: { '_auth_data_custom.id': 1 },
419+
_auth_data_facebook_id: { '_auth_data_facebook.id': 1 },
420+
_auth_data_myoauth_id: { '_auth_data_myoauth.id': 1 },
421+
_auth_data_shortLivedAuth_id: { '_auth_data_shortLivedAuth.id': 1 },
422+
_auth_data_anonymous_id: { '_auth_data_anonymous.id': 1 },
418423
});
419424
}
420425
);
@@ -441,6 +446,11 @@ describe('DatabaseController', function () {
441446
email_1: { email: 1 },
442447
_email_verify_token: { _email_verify_token: 1 },
443448
_perishable_token: { _perishable_token: 1 },
449+
_auth_data_custom_id: { '_auth_data_custom.id': 1 },
450+
_auth_data_facebook_id: { '_auth_data_facebook.id': 1 },
451+
_auth_data_myoauth_id: { '_auth_data_myoauth.id': 1 },
452+
_auth_data_shortLivedAuth_id: { '_auth_data_shortLivedAuth.id': 1 },
453+
_auth_data_anonymous_id: { '_auth_data_anonymous.id': 1 },
444454
});
445455
}
446456
);

‎spec/ParseUser.spec.js‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ describe('Parse.User testing', () => {
390390
expect(newUser).not.toBeUndefined();
391391
});
392392

393-
it('should be let masterKey lock user out with authData', async () => {
393+
it_only_db('mongo')('should reject duplicate authData when masterKey locks user out (mongo)', async () => {
394394
const response = await request({
395395
method: 'POST',
396396
url: 'http://localhost:8378/1/classes/_User',
@@ -406,15 +406,13 @@ describe('Parse.User testing', () => {
406406
});
407407
const body = response.data;
408408
const objectId = body.objectId;
409-
const sessionToken = body.sessionToken;
410-
expect(sessionToken).toBeDefined();
409+
expect(body.sessionToken).toBeDefined();
411410
expect(objectId).toBeDefined();
412411
const user = new Parse.User();
413412
user.id = objectId;
414413
const ACL = new Parse.ACL();
415414
user.setACL(ACL);
416415
await user.save(null, { useMasterKey: true });
417-
// update the user
418416
const options = {
419417
method: 'POST',
420418
url: `http://localhost:8378/1/classes/_User/`,
@@ -430,8 +428,61 @@ describe('Parse.User testing', () => {
430428
},
431429
},
432430
};
433-
const res = await request(options);
434-
expect(res.data.objectId).not.toEqual(objectId);
431+
try {
432+
await request(options);
433+
fail('should have thrown');
434+
} catch (err) {
435+
expect(err.data.code).toBe(208);
436+
expect(err.data.error).toBe('this auth is already used');
437+
}
438+
});
439+
440+
it_only_db('postgres')('should reject duplicate authData when masterKey locks user out (postgres)', async () => {
441+
await reconfigureServer();
442+
const response = await request({
443+
method: 'POST',
444+
url: 'http://localhost:8378/1/classes/_User',
445+
headers: {
446+
'X-Parse-Application-Id': Parse.applicationId,
447+
'X-Parse-REST-API-Key': 'rest',
448+
'Content-Type': 'application/json',
449+
},
450+
body: {
451+
key: 'value',
452+
authData: { anonymous: { id: '00000000-0000-0000-0000-000000000001' } },
453+
},
454+
});
455+
const body = response.data;
456+
const objectId = body.objectId;
457+
expect(body.sessionToken).toBeDefined();
458+
expect(objectId).toBeDefined();
459+
const user = new Parse.User();
460+
user.id = objectId;
461+
const ACL = new Parse.ACL();
462+
user.setACL(ACL);
463+
await user.save(null, { useMasterKey: true });
464+
const options = {
465+
method: 'POST',
466+
url: `http://localhost:8378/1/classes/_User/`,
467+
headers: {
468+
'X-Parse-Application-Id': Parse.applicationId,
469+
'X-Parse-REST-API-Key': 'rest',
470+
'Content-Type': 'application/json',
471+
},
472+
body: {
473+
key: 'otherValue',
474+
authData: {
475+
anonymous: { id: '00000000-0000-0000-0000-000000000001' },
476+
},
477+
},
478+
};
479+
try {
480+
await request(options);
481+
fail('should have thrown');
482+
} catch (err) {
483+
expect(err.data.code).toBe(208);
484+
expect(err.data.error).toBe('this auth is already used');
485+
}
435486
});
436487

437488
it('user login with files', done => {

‎src/Adapters/Storage/Mongo/MongoStorageAdapter.js‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,13 @@ export class MongoStorageAdapter implements StorageAdapter {
582582
if (matches && Array.isArray(matches)) {
583583
err.userInfo = { duplicated_field: matches[1] };
584584
}
585+
// Check for authData unique index violations
586+
if (!err.userInfo) {
587+
const authDataMatch = error.message.match(/index:\s+(_auth_data_[a-zA-Z0-9_]+_id)/);
588+
if (authDataMatch) {
589+
err.userInfo = { duplicated_field: authDataMatch[1] };
590+
}
591+
}
585592
}
586593
throw err;
587594
}
@@ -658,10 +665,26 @@ export class MongoStorageAdapter implements StorageAdapter {
658665
.catch(error => {
659666
if (error.code === 11000) {
660667
logger.error('Duplicate key error:', error.message);
661-
throw new Parse.Error(
668+
const err = new Parse.Error(
662669
Parse.Error.DUPLICATE_VALUE,
663670
'A duplicate value for a field with unique values was provided'
664671
);
672+
err.underlyingError = error;
673+
if (error.message) {
674+
const matches = error.message.match(
675+
/index:[\sa-zA-Z0-9_\-\.]+\$?([a-zA-Z_-]+)_1/
676+
);
677+
if (matches && Array.isArray(matches)) {
678+
err.userInfo = { duplicated_field: matches[1] };
679+
}
680+
if (!err.userInfo) {
681+
const authDataMatch = error.message.match(/index:\s+(_auth_data_[a-zA-Z0-9_]+_id)/);
682+
if (authDataMatch) {
683+
err.userInfo = { duplicated_field: authDataMatch[1] };
684+
}
685+
}
686+
}
687+
throw err;
665688
}
666689
throw error;
667690
})
@@ -818,6 +841,32 @@ export class MongoStorageAdapter implements StorageAdapter {
818841
.catch(err => this.handleError(err));
819842
}
820843

844+
// Creates a unique sparse index on _auth_data_<provider>.id to prevent
845+
// race conditions during concurrent signups with the same authData.
846+
ensureAuthDataUniqueness(provider: string) {
847+
return this._adaptiveCollection('_User')
848+
.then(collection =>
849+
collection._mongoCollection.createIndex(
850+
{ [`_auth_data_${provider}.id`]: 1 },
851+
{ unique: true, sparse: true, background: true, name: `_auth_data_${provider}_id` }
852+
)
853+
)
854+
.catch(error => {
855+
if (error.code === 11000) {
856+
throw new Parse.Error(
857+
Parse.Error.DUPLICATE_VALUE,
858+
'Tried to ensure field uniqueness for a class that already has duplicates.'
859+
);
860+
}
861+
// Ignore "index already exists with same name" or "index already exists with different options"
862+
if (error.code === 85 || error.code === 86) {
863+
return;
864+
}
865+
throw error;
866+
})
867+
.catch(err => this.handleError(err));
868+
}
869+
821870
// Used in tests
822871
_rawFind(className: string, query: QueryType) {
823872
return this._adaptiveCollection(className)

0 commit comments

Comments
 (0)