Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 19, 2024
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b492a9
feat: dry handleAuthData for safer code maintenance in the futur
Moumouls Mar 17, 2024
c00af84
Merge branch 'alpha' into moumouls/dry-handle-auth-data
mtrezza Mar 19, 2024
be15411
Update spec/ParseUser.spec.js
mtrezza Mar 19, 2024
f78357a
Apply suggestions from code review
mtrezza Mar 19, 2024
bd88aa9
Apply suggestions from code review
mtrezza Mar 19, 2024
a5b97d0
Merge branch 'alpha' into moumouls/dry-handle-auth-data
mtrezza Mar 19, 2024
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
View file
Open in desktop
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(() => { });
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(() => { });
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(() => { });
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
16 changes: 8 additions & 8 deletions spec/ParseUser.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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(),
};
Expand Down Expand Up @@ -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();
}
}
Expand Down
18 changes: 8 additions & 10 deletions src/RestWrite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

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

Expand Down Expand Up @@ -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,
Expand Down