From 1b492a92436678a922c158cd3486db2757372da4 Mon Sep 17 00:00:00 2001 From: Antoine Cormouls Date: Sun, 17 Mar 2024 20:06:07 +0100 Subject: [PATCH 1/4] feat: dry handleAuthData for safer code maintenance in the futur --- spec/AuthenticationAdaptersV2.spec.js | 35 ++++++++++++++++++++++++--- spec/ParseUser.spec.js | 16 ++++++------ src/RestWrite.js | 18 ++++++-------- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/spec/AuthenticationAdaptersV2.spec.js b/spec/AuthenticationAdaptersV2.spec.js index aaa172ea66..7c94a601f5 100644 --- a/spec/AuthenticationAdaptersV2.spec.js +++ b/spec/AuthenticationAdaptersV2.spec.js @@ -487,6 +487,33 @@ describe('Auth Adapter features', () => { expect(baseAdapter2.validateAuthData).toHaveBeenCalledTimes(2); }); + it('should not perform authData validation twice when data mutated', async () => { + spyOn(baseAdapter, 'validateAuthData').and.resolveTo({}); + await reconfigureServer({ + auth: { baseAdapter }, + allowExpiredAuthDataToken: false, + }); + + const user = new Parse.User(); + + await user.save({ + authData: { + baseAdapter: { id: 'baseAdapter', token: "sometoken1" }, + }, + }); + + expect(baseAdapter.validateAuthData).toHaveBeenCalledTimes(1); + + const user2 = new Parse.User(); + await user2.save({ + authData: { + baseAdapter: { id: 'baseAdapter', token: "sometoken2" }, + }, + }); + + expect(baseAdapter.validateAuthData).toHaveBeenCalledTimes(2); + }); + it('should require additional provider if configured', async () => { await reconfigureServer({ auth: { baseAdapter, additionalAdapter }, @@ -860,7 +887,7 @@ describe('Auth Adapter features', () => { auth: { challengeAdapter: throwInChallengeAdapter }, }); let logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => {}); + spyOn(logger, 'error').and.callFake(() => { }); await expectAsync( requestWithExpectedError({ headers: headers, @@ -885,7 +912,7 @@ describe('Auth Adapter features', () => { await reconfigureServer({ auth: { modernAdapter: throwInSetup } }); logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => {}); + spyOn(logger, 'error').and.callFake(() => { }); let user = new Parse.User(); await expectAsync( user.save({ authData: { modernAdapter: { id: 'modernAdapter' } } }) @@ -907,7 +934,7 @@ describe('Auth Adapter features', () => { await reconfigureServer({ auth: { modernAdapter: throwInUpdate } }); logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => {}); + spyOn(logger, 'error').and.callFake(() => { }); user = new Parse.User(); await user.save({ authData: { modernAdapter: { id: 'updateAdapter' } } }); await expectAsync( @@ -937,7 +964,7 @@ describe('Auth Adapter features', () => { allowExpiredAuthDataToken: false, }); logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => {}); + spyOn(logger, 'error').and.callFake(() => { }); user = new Parse.User(); await user.save({ authData: { modernAdapter: { id: 'modernAdapter' } } }); const user2 = new Parse.User(); diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index e97db08a6c..92d4d3c4c2 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -3350,7 +3350,7 @@ describe('Parse.User testing', () => { it('should not allow updates to emailVerified', done => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3386,7 +3386,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on GET users/me (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3429,7 +3429,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on GET users/id (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3477,7 +3477,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on login (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3521,7 +3521,7 @@ describe('Parse.User testing', () => { it('should not allow updates to hidden fields', async () => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3546,7 +3546,7 @@ describe('Parse.User testing', () => { it('should allow updates to fields with maintenanceKey', async () => { const emailAdapter = { - sendVerificationEmail: () => {}, + sendVerificationEmail: () => { }, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3576,8 +3576,8 @@ describe('Parse.User testing', () => { expect(e.code).toBe(Parse.Error.OBJECT_NOT_FOUND); expect( e.message === 'Invalid username/password.' || - e.message === - 'Your account is locked due to multiple failed login attempts. Please try again after 1 minute(s)' + e.message === + 'Your account is locked due to multiple failed login attempts. Please try again after 1 minute(s)' ).toBeTrue(); } } diff --git a/src/RestWrite.js b/src/RestWrite.js index b93bb2d0c8..a8541bdcf8 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -523,10 +523,15 @@ RestWrite.prototype.handleAuthData = async function (authData) { const r = await Auth.findUsersWithAuthData(this.config, authData); const results = this.filteredObjectsByACL(r); - if (results.length > 1) { + const userId = this.getUserId(); + const userResult = results[0]; + + const foundUserIsNotCurrentUser = userId && userResult && userId !== userResult.objectId; + + if (results.length > 1 || foundUserIsNotCurrentUser) { // To avoid https://github.com/parse-community/parse-server/security/advisories/GHSA-8w3j-g983-8jh5 // Let's run some validation before throwing - await Auth.handleAuthDataValidation(authData, this, results[0]); + await Auth.handleAuthDataValidation(authData, this, userResult); throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); } @@ -544,13 +549,6 @@ RestWrite.prototype.handleAuthData = async function (authData) { // User found with provided authData if (results.length === 1) { - const userId = this.getUserId(); - const userResult = results[0]; - // Prevent duplicate authData id - if (userId && userId !== userResult.objectId) { - await Auth.handleAuthDataValidation(authData, this, results[0]); - throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); - } this.storage.authProvider = Object.keys(authData).join(','); @@ -1311,7 +1309,7 @@ RestWrite.prototype.handleInstallation = function () { throw new Parse.Error( 132, 'Must specify installationId when deviceToken ' + - 'matches multiple Installation objects' + 'matches multiple Installation objects' ); } else { // Multiple device token matches and we specified an installation ID, From be154112511672a0c558502d0a9ff6f99a1bf9ba Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Tue, 19 Mar 2024 22:28:54 +0100 Subject: [PATCH 2/4] Update spec/ParseUser.spec.js Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/ParseUser.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 92d4d3c4c2..4b4b2a4f77 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -3350,7 +3350,7 @@ describe('Parse.User testing', () => { it('should not allow updates to emailVerified', done => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; From f78357a4e19c7d8e5389bb31a9dac314ed33e282 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Tue, 19 Mar 2024 22:30:53 +0100 Subject: [PATCH 3/4] Apply suggestions from code review Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/ParseUser.spec.js | 14 +++++++------- src/RestWrite.js | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 4b4b2a4f77..e97db08a6c 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -3386,7 +3386,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on GET users/me (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3429,7 +3429,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on GET users/id (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3477,7 +3477,7 @@ describe('Parse.User testing', () => { it('should not retrieve hidden fields on login (#3432)', done => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3521,7 +3521,7 @@ describe('Parse.User testing', () => { it('should not allow updates to hidden fields', async () => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3546,7 +3546,7 @@ describe('Parse.User testing', () => { it('should allow updates to fields with maintenanceKey', async () => { const emailAdapter = { - sendVerificationEmail: () => { }, + sendVerificationEmail: () => {}, sendPasswordResetEmail: () => Promise.resolve(), sendMail: () => Promise.resolve(), }; @@ -3576,8 +3576,8 @@ describe('Parse.User testing', () => { expect(e.code).toBe(Parse.Error.OBJECT_NOT_FOUND); expect( e.message === 'Invalid username/password.' || - e.message === - 'Your account is locked due to multiple failed login attempts. Please try again after 1 minute(s)' + e.message === + 'Your account is locked due to multiple failed login attempts. Please try again after 1 minute(s)' ).toBeTrue(); } } diff --git a/src/RestWrite.js b/src/RestWrite.js index a8541bdcf8..c2d4e1580c 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -525,7 +525,6 @@ RestWrite.prototype.handleAuthData = async function (authData) { const userId = this.getUserId(); const userResult = results[0]; - const foundUserIsNotCurrentUser = userId && userResult && userId !== userResult.objectId; if (results.length > 1 || foundUserIsNotCurrentUser) { @@ -1309,7 +1308,7 @@ RestWrite.prototype.handleInstallation = function () { throw new Parse.Error( 132, 'Must specify installationId when deviceToken ' + - 'matches multiple Installation objects' + 'matches multiple Installation objects' ); } else { // Multiple device token matches and we specified an installation ID, From bd88aa9ddfcef8f587e852d57f6fd015bdebec87 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Tue, 19 Mar 2024 22:31:56 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/AuthenticationAdaptersV2.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/AuthenticationAdaptersV2.spec.js b/spec/AuthenticationAdaptersV2.spec.js index 7c94a601f5..41c76b1f2b 100644 --- a/spec/AuthenticationAdaptersV2.spec.js +++ b/spec/AuthenticationAdaptersV2.spec.js @@ -887,7 +887,7 @@ describe('Auth Adapter features', () => { auth: { challengeAdapter: throwInChallengeAdapter }, }); let logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => { }); + spyOn(logger, 'error').and.callFake(() => {}); await expectAsync( requestWithExpectedError({ headers: headers, @@ -912,7 +912,7 @@ describe('Auth Adapter features', () => { await reconfigureServer({ auth: { modernAdapter: throwInSetup } }); logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => { }); + spyOn(logger, 'error').and.callFake(() => {}); let user = new Parse.User(); await expectAsync( user.save({ authData: { modernAdapter: { id: 'modernAdapter' } } }) @@ -934,7 +934,7 @@ describe('Auth Adapter features', () => { await reconfigureServer({ auth: { modernAdapter: throwInUpdate } }); logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => { }); + spyOn(logger, 'error').and.callFake(() => {}); user = new Parse.User(); await user.save({ authData: { modernAdapter: { id: 'updateAdapter' } } }); await expectAsync(