Skip to content

Commit 7669bd9

Browse files
committed
Optimize authData handling logic and integrate delta-based updates
Refactor authData processing for improved clarity, efficiency, and correctness. Introduce `diffAuthData` and `subsetEqual` to handle partial updates, unlinking, and change detection intelligently. Update `RestWrite` logic to utilize refined authData delta behavior, preventing redundant operations. Add comprehensive test coverage for edge cases and optimizations. Refactor existing password policy validations for readability.
1 parent 82fdb0d commit 7669bd9

File tree

4 files changed

+528
-108
lines changed

4 files changed

+528
-108
lines changed

spec/AuthenticationAdaptersV2.spec.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ describe('Auth Adapter features', () => {
829829
expect(firstCall[2].user.id).toEqual(user.id);
830830
expect(firstCall.length).toEqual(3);
831831

832+
payload.someData = false;
832833
await user.save({ authData: { baseAdapter2: payload } }, { useMasterKey: true });
833834

834835
const secondCall = baseAdapter2.validateAuthData.calls.argsFor(1);

spec/Users.authdata.spec.js

Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
describe('AuthData Delta Behavior', () => {
2+
const MOCK_USER_ID = 'mockUserId';
3+
const MOCK_ACCESS_TOKEN = 'mockAccessToken123';
4+
5+
const createMockUser = () => ({
6+
id: MOCK_USER_ID,
7+
code: 'C1'
8+
});
9+
10+
const mockGooglePlayGamesAPI = () => {
11+
mockFetch([
12+
{
13+
url: 'https://oauth2.googleapis.com/token',
14+
method: 'POST',
15+
response: {
16+
ok: true,
17+
json: () => Promise.resolve({ access_token: MOCK_ACCESS_TOKEN }),
18+
},
19+
},
20+
{
21+
url: `https://www.googleapis.com/games/v1/players/${MOCK_USER_ID}`,
22+
method: 'GET',
23+
response: {
24+
ok: true,
25+
json: () => Promise.resolve({ playerId: MOCK_USER_ID }),
26+
},
27+
},
28+
]);
29+
};
30+
31+
const setupAuthConfig = (additionalProviders = {}) => {
32+
return reconfigureServer({
33+
auth: {
34+
gpgames: {
35+
clientId: 'validClientId',
36+
clientSecret: 'validClientSecret',
37+
},
38+
someAdapter1: {
39+
validateAuthData: () => Promise.resolve(),
40+
validateAppId: () => Promise.resolve(),
41+
validateOptions: () => {},
42+
},
43+
someAdapter2: {
44+
validateAuthData: () => Promise.resolve(),
45+
validateAppId: () => Promise.resolve(),
46+
validateOptions: () => {},
47+
},
48+
...additionalProviders,
49+
},
50+
});
51+
};
52+
53+
beforeEach(async () => {
54+
await setupAuthConfig();
55+
});
56+
57+
describe('Provider Linking', () => {
58+
it('should link someAdapter1 without affecting unchanged Google Play Games auth', async () => {
59+
mockGooglePlayGamesAPI();
60+
61+
const authData = createMockUser();
62+
const user = await Parse.User.logInWith('gpgames', { authData });
63+
const sessionToken = user.getSessionToken();
64+
65+
await user.fetch({ sessionToken });
66+
const currentAuthData = user.get('authData') || {};
67+
68+
user.set('authData', {
69+
...currentAuthData,
70+
someAdapter1: { id: 'T1', access_token: 'token123' },
71+
});
72+
await user.save(null, { sessionToken });
73+
74+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
75+
const finalAuthData = updatedUser.get('authData');
76+
77+
expect(finalAuthData.gpgames?.id).toBe(MOCK_USER_ID);
78+
expect(finalAuthData.someAdapter1?.id).toBe('T1');
79+
});
80+
81+
it('should handle multiple providers correctly', async () => {
82+
mockGooglePlayGamesAPI();
83+
84+
const authData = {
85+
gpgames: { id: MOCK_USER_ID, code: 'C4' },
86+
someAdapter2: { id: 'F1', access_token: 'fb_token' },
87+
};
88+
89+
const user = new Parse.User();
90+
user.set('authData', authData);
91+
await user.save();
92+
93+
const sessionToken = user.getSessionToken();
94+
95+
await user.fetch({ sessionToken });
96+
const currentAuthData = user.get('authData') || {};
97+
98+
user.set('authData', {
99+
someAdapter2: currentAuthData.someAdapter2,
100+
someAdapter1: { id: 'T2', access_token: 'tw_token' },
101+
gpgames: null, // Unlink Google Play Games
102+
});
103+
await user.save(null, { sessionToken });
104+
105+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
106+
const finalAuthData = updatedUser.get('authData') || {};
107+
108+
expect(finalAuthData.gpgames).toBeUndefined();
109+
expect(finalAuthData.someAdapter2?.id).toBe('F1');
110+
expect(finalAuthData.someAdapter1?.id).toBe('T2');
111+
});
112+
});
113+
114+
describe('Provider Unlinking', () => {
115+
it('should unlink provider via null', async () => {
116+
mockGooglePlayGamesAPI();
117+
118+
const authData = createMockUser();
119+
const user = await Parse.User.logInWith('gpgames', { authData });
120+
const sessionToken = user.getSessionToken();
121+
122+
await user.fetch({ sessionToken });
123+
const currentAuthData = user.get('authData') || {};
124+
125+
user.set('authData', {
126+
...currentAuthData,
127+
gpgames: null,
128+
});
129+
await user.save(null, { sessionToken });
130+
131+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
132+
const finalAuthData = updatedUser.get('authData') || {};
133+
134+
expect(finalAuthData.gpgames).toBeUndefined();
135+
});
136+
});
137+
138+
describe('Data Validation Optimization', () => {
139+
it('should skip revalidation when authData is identical', async () => {
140+
mockGooglePlayGamesAPI();
141+
142+
const authData = createMockUser();
143+
const user = await Parse.User.logInWith('gpgames', { authData });
144+
const sessionToken = user.getSessionToken();
145+
146+
await user.fetch({ sessionToken });
147+
const existingAuthData = user.get('authData');
148+
149+
// Small delay to ensure timestamp differences don't affect comparison
150+
await new Promise(resolve => setTimeout(resolve, 100));
151+
152+
user.set('authData', JSON.parse(JSON.stringify(existingAuthData)));
153+
await user.save(null, { sessionToken });
154+
155+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
156+
const finalAuthData = updatedUser.get('authData') || {};
157+
158+
expect(finalAuthData.gpgames?.id).toBe(MOCK_USER_ID);
159+
});
160+
161+
it('should handle empty authData gracefully', async () => {
162+
mockGooglePlayGamesAPI();
163+
164+
const user = await Parse.User.signUp('test', 'password123');
165+
166+
const sessionToken = user.getSessionToken();
167+
await user.fetch({ sessionToken });
168+
169+
user.set('authData', {
170+
someAdapter1: { id: 'T3', access_token: 'token456' },
171+
});
172+
await user.save(null, { sessionToken });
173+
174+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
175+
const finalAuthData = updatedUser.get('authData');
176+
177+
expect(finalAuthData).toBeDefined();
178+
expect(finalAuthData.someAdapter1?.id).toBe('T3');
179+
});
180+
});
181+
182+
describe('Partial Data Updates', () => {
183+
it('should handle partial provider data updates correctly', async () => {
184+
mockGooglePlayGamesAPI()
185+
186+
const authData = createMockUser();
187+
const user = await Parse.User.logInWith('gpgames', { authData });
188+
189+
const sessionToken = user.getSessionToken();
190+
191+
await user.fetch({ sessionToken });
192+
193+
const currentAuthData = user.get('authData') || {};
194+
user.set('authData', {
195+
...currentAuthData,
196+
gpgames: {
197+
...currentAuthData.gpgames,
198+
code: 'new',
199+
},
200+
});
201+
await user.save(null, { sessionToken });
202+
203+
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
204+
const finalAuthData = updatedUser.get('authData');
205+
206+
expect(finalAuthData.gpgames.id).toBe(MOCK_USER_ID);
207+
});
208+
});
209+
210+
describe('API Call Optimization', () => {
211+
beforeEach(async () => {
212+
await setupAuthConfig();
213+
});
214+
215+
it('should not call getAccessTokenFromCode for unchanged authData', async () => {
216+
mockGooglePlayGamesAPI();
217+
218+
const authData = createMockUser();
219+
const user = await Parse.User.logInWith('gpgames', { authData });
220+
const sessionToken = user.getSessionToken();
221+
222+
const initialCallCount = global.fetch.calls.count();
223+
224+
const freshUser = await new Parse.Query(Parse.User).get(user.id, { sessionToken });
225+
const currentAuthData = freshUser.get('authData');
226+
227+
freshUser.set('authData', JSON.parse(JSON.stringify(currentAuthData)));
228+
await freshUser.save(null, { sessionToken });
229+
230+
expect(global.fetch.calls.count()).toBe(initialCallCount);
231+
});
232+
233+
it('should handle mixed authData operations without redundant API calls', async () => {
234+
mockGooglePlayGamesAPI();
235+
236+
const authData = createMockUser();
237+
const user = await Parse.User.logInWith('gpgames', { authData });
238+
const sessionToken = user.getSessionToken();
239+
240+
const initialCallCount = global.fetch.calls.count();
241+
242+
const freshUser = await new Parse.Query(Parse.User).get(user.id, { sessionToken });
243+
const currentAuthData = freshUser.get('authData') || {};
244+
245+
freshUser.set('authData', {
246+
...currentAuthData,
247+
someAdapter2: { id: 'fb123', access_token: 'fb_token' }
248+
});
249+
await freshUser.save(null, { sessionToken });
250+
251+
expect(global.fetch.calls.count()).toBe(initialCallCount);
252+
253+
const finalUser = await new Parse.Query(Parse.User).get(user.id, { sessionToken });
254+
const finalAuthData = finalUser.get('authData') || {};
255+
256+
expect(finalAuthData.gpgames?.id).toBe(MOCK_USER_ID);
257+
expect(finalAuthData.someAdapter2?.id).toBe('fb123');
258+
});
259+
});
260+
});

src/Auth.js

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,10 +423,15 @@ const findUsersWithAuthData = async (config, authData, beforeFind) => {
423423
const queries = await Promise.all(
424424
providers.map(async provider => {
425425
const providerAuthData = authData[provider];
426+
if (!providerAuthData) {
427+
return null;
428+
}
426429

427-
const adapter = config.authDataManager.getValidatorForProvider(provider)?.adapter;
428-
if (beforeFind && typeof adapter?.beforeFind === 'function') {
429-
await adapter.beforeFind(providerAuthData);
430+
if (beforeFind) {
431+
const adapter = config.authDataManager.getValidatorForProvider(provider)?.adapter;
432+
if (typeof adapter?.beforeFind === 'function') {
433+
await adapter.beforeFind(providerAuthData);
434+
}
430435
}
431436

432437
if (!providerAuthData?.id) {
@@ -601,6 +606,70 @@ const handleAuthDataValidation = async (authData, req, foundUser) => {
601606
return acc;
602607
};
603608

609+
const subsetEqual = (prev, next) => {
610+
if (prev === next) return true;
611+
if (prev == null || next == null) return false;
612+
613+
const tp = typeof prev;
614+
const tn = typeof next;
615+
if (tn !== 'object' || tp !== 'object') return prev === next;
616+
617+
// arrays: require element-wise equality for the provided portion
618+
if (Array.isArray(next)) {
619+
if (!Array.isArray(prev)) return false;
620+
if (next.length !== prev.length) return false;
621+
for (let i = 0; i < next.length; i++) {
622+
if (!subsetEqual(prev[i], next[i])) return false;
623+
}
624+
return true;
625+
}
626+
627+
// objects: every provided key in `next` must match `prev`
628+
for (const k of Object.keys(next)) {
629+
const nv = next[k];
630+
if (typeof nv === 'undefined') continue; // treat "not provided" as no-op
631+
const pv = prev[k];
632+
if (!subsetEqual(pv, nv)) return false;
633+
}
634+
return true;
635+
}
636+
637+
/**
638+
* Delta between current and incoming authData with partial update semantics:
639+
* - changed: providers truly changed (new or value differs on provided keys)
640+
* - unlink: providers explicitly set to null (remove without validation)
641+
* - unchanged: providers either absent in incoming or provided as matching subset
642+
*/
643+
const diffAuthData = (current = {}, incoming = {}) => {
644+
const changed = {};
645+
const unlink = {};
646+
const unchanged = {};
647+
648+
const providers = new Set([...Object.keys(current), ...Object.keys(incoming)]);
649+
for (const p of providers) {
650+
const prev = current[p];
651+
const next = incoming[p];
652+
653+
if (next === null) { unlink[p] = true; continue; }
654+
if (typeof next === 'undefined') { // provider untouched
655+
if (typeof prev !== 'undefined') unchanged[p] = prev;
656+
continue;
657+
}
658+
if (typeof prev === 'undefined') { // new provider
659+
changed[p] = next;
660+
continue;
661+
}
662+
663+
// key point: treat sanitized partial payload (subset) as unchanged
664+
if (subsetEqual(prev, next)) {
665+
unchanged[p] = prev;
666+
} else {
667+
changed[p] = next;
668+
}
669+
}
670+
return { changed, unlink, unchanged };
671+
};
672+
604673
module.exports = {
605674
Auth,
606675
master,
@@ -614,4 +683,6 @@ module.exports = {
614683
hasMutatedAuthData,
615684
checkIfUserHasProvidedConfiguredProvidersForLogin,
616685
handleAuthDataValidation,
686+
subsetEqual,
687+
diffAuthData
617688
};

0 commit comments

Comments
 (0)