From 1b492a92436678a922c158cd3486db2757372da4 Mon Sep 17 00:00:00 2001 From: Antoine Cormouls Date: Sun, 17 Mar 2024 20:06:07 +0100 Subject: [PATCH] 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,