Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Dry handleAuthData for safer code maintenance in the future #9025

Merged
merged 6 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions spec/AuthenticationAdaptersV2.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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(() => { });
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
await expectAsync(
requestWithExpectedError({
headers: headers,
Expand All @@ -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(() => { });
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
let user = new Parse.User();
await expectAsync(
user.save({ authData: { modernAdapter: { id: 'modernAdapter' } } })
Expand All @@ -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(() => { });
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
user = new Parse.User();
await user.save({ authData: { modernAdapter: { id: 'updateAdapter' } } });
await expectAsync(
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 6 additions & 9 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,14 @@ 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');
}

Expand All @@ -544,13 +548,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(',');

Expand Down
Loading