Skip to content

Commit 4c42259

Browse files
committed
fix: address CodeRabbit review feedback
1 parent 2d2b2fa commit 4c42259

File tree

2 files changed

+66
-63
lines changed

2 files changed

+66
-63
lines changed

spec/vulnerabilities.spec.js

Lines changed: 62 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,80 +4238,80 @@ describe('(GHSA-g4cf-xj29-wqqr) DoS via unindexed database query for unconfigure
42384238
});
42394239
expect(authDataQueries.length).toBeGreaterThan(0);
42404240
});
4241+
});
42414242

4242-
describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => {
4243-
const mfaHeaders = {
4244-
'X-Parse-Application-Id': 'test',
4245-
'X-Parse-REST-API-Key': 'rest',
4246-
'Content-Type': 'application/json',
4247-
};
4243+
describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => {
4244+
const mfaHeaders = {
4245+
'X-Parse-Application-Id': 'test',
4246+
'X-Parse-REST-API-Key': 'rest',
4247+
'Content-Type': 'application/json',
4248+
};
42484249

4249-
beforeEach(async () => {
4250-
await reconfigureServer({
4251-
auth: {
4252-
mfa: {
4253-
enabled: true,
4254-
options: ['TOTP'],
4255-
algorithm: 'SHA1',
4256-
digits: 6,
4257-
period: 30,
4258-
},
4250+
beforeEach(async () => {
4251+
await reconfigureServer({
4252+
auth: {
4253+
mfa: {
4254+
enabled: true,
4255+
options: ['TOTP'],
4256+
algorithm: 'SHA1',
4257+
digits: 6,
4258+
period: 30,
42594259
},
4260-
});
4260+
},
42614261
});
4262+
});
42624263

4263-
it('rejects concurrent logins using the same MFA recovery code', async () => {
4264-
const OTPAuth = require('otpauth');
4265-
const user = await Parse.User.signUp('mfauser', 'password123');
4266-
const secret = new OTPAuth.Secret();
4267-
const totp = new OTPAuth.TOTP({
4268-
algorithm: 'SHA1',
4269-
digits: 6,
4270-
period: 30,
4271-
secret,
4272-
});
4273-
const token = totp.generate();
4274-
await user.save(
4275-
{ authData: { mfa: { secret: secret.base32, token } } },
4276-
{ sessionToken: user.getSessionToken() }
4277-
);
4264+
it('rejects concurrent logins using the same MFA recovery code', async () => {
4265+
const OTPAuth = require('otpauth');
4266+
const user = await Parse.User.signUp('mfauser', 'password123');
4267+
const secret = new OTPAuth.Secret();
4268+
const totp = new OTPAuth.TOTP({
4269+
algorithm: 'SHA1',
4270+
digits: 6,
4271+
period: 30,
4272+
secret,
4273+
});
4274+
const token = totp.generate();
4275+
await user.save(
4276+
{ authData: { mfa: { secret: secret.base32, token } } },
4277+
{ sessionToken: user.getSessionToken() }
4278+
);
42784279

4279-
// Get recovery codes from stored auth data
4280-
await user.fetch({ useMasterKey: true });
4281-
const recoveryCode = user.get('authData').mfa.recovery[0];
4282-
expect(recoveryCode).toBeDefined();
4280+
// Get recovery codes from stored auth data
4281+
await user.fetch({ useMasterKey: true });
4282+
const recoveryCode = user.get('authData').mfa.recovery[0];
4283+
expect(recoveryCode).toBeDefined();
42834284

4284-
// Send concurrent login requests with the same recovery code
4285-
const loginWithRecovery = () =>
4286-
request({
4287-
method: 'POST',
4288-
url: 'http://localhost:8378/1/login',
4289-
headers: mfaHeaders,
4290-
body: JSON.stringify({
4291-
username: 'mfauser',
4292-
password: 'password123',
4293-
authData: {
4294-
mfa: {
4295-
token: recoveryCode,
4296-
},
4285+
// Send concurrent login requests with the same recovery code
4286+
const loginWithRecovery = () =>
4287+
request({
4288+
method: 'POST',
4289+
url: 'http://localhost:8378/1/login',
4290+
headers: mfaHeaders,
4291+
body: JSON.stringify({
4292+
username: 'mfauser',
4293+
password: 'password123',
4294+
authData: {
4295+
mfa: {
4296+
token: recoveryCode,
42974297
},
4298-
}),
4299-
});
4298+
},
4299+
}),
4300+
});
43004301

4301-
const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery()));
4302+
const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery()));
43024303

4303-
const succeeded = results.filter(r => r.status === 'fulfilled');
4304-
const failed = results.filter(r => r.status === 'rejected');
4304+
const succeeded = results.filter(r => r.status === 'fulfilled');
4305+
const failed = results.filter(r => r.status === 'rejected');
43054306

4306-
// Exactly one request should succeed; all others should fail
4307-
expect(succeeded.length).toBe(1);
4308-
expect(failed.length).toBe(9);
4307+
// Exactly one request should succeed; all others should fail
4308+
expect(succeeded.length).toBe(1);
4309+
expect(failed.length).toBe(9);
43094310

4310-
// Verify the recovery code has been consumed
4311-
await user.fetch({ useMasterKey: true });
4312-
const remainingRecovery = user.get('authData').mfa.recovery;
4313-
expect(remainingRecovery).not.toContain(recoveryCode);
4314-
});
4311+
// Verify the recovery code has been consumed
4312+
await user.fetch({ useMasterKey: true });
4313+
const remainingRecovery = user.get('authData').mfa.recovery;
4314+
expect(remainingRecovery).not.toContain(recoveryCode);
43154315
});
43164316
});
43174317

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,10 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus
340340
patterns.push(`$${index}:raw = $${index + 1}::text`);
341341
values.push(name, fieldValue);
342342
index += 2;
343-
} else if (typeof fieldValue === 'object') {
343+
} else if (
344+
typeof fieldValue === 'object' &&
345+
!Object.keys(fieldValue).some(key => key.startsWith('$'))
346+
) {
344347
name = transformDotFieldToComponents(fieldName).join('->');
345348
patterns.push(`($${index}:raw)::jsonb = $${index + 1}::jsonb`);
346349
values.push(name, JSON.stringify(fieldValue));

0 commit comments

Comments
 (0)