Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
208 changes: 203 additions & 5 deletions spec/CloudCode.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3307,19 +3307,19 @@ describe('afterFind hooks', () => {
}).not.toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
expect(() => {
Parse.Cloud.beforeLogin('SomeClass', () => { });
}).toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
}).toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.afterLogin(() => { });
}).not.toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
}).not.toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.afterLogin('_User', () => { });
}).not.toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
}).not.toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.afterLogin(Parse.User, () => { });
}).not.toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
}).not.toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.afterLogin('SomeClass', () => { });
}).toThrow('Only the _User class is allowed for the beforeLogin and afterLogin triggers');
}).toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.afterLogout(() => { });
}).not.toThrow();
Expand Down Expand Up @@ -4656,3 +4656,201 @@ describe('sendEmail', () => {
);
});
});

describe('beforePasswordResetRequest hook', () => {
it('should run beforePasswordResetRequest with valid user', async done => {
let hit = 0;
let sendPasswordResetEmailCalled = false;
const emailAdapter = {
sendVerificationEmail: () => Promise.resolve(),
sendPasswordResetEmail: () => {
sendPasswordResetEmailCalled = true;
},
sendMail: () => {},
};

await reconfigureServer({
appName: 'test',
emailAdapter: emailAdapter,
publicServerURL: 'http://localhost:8378/1',
});

Parse.Cloud.beforePasswordResetRequest(req => {
hit++;
expect(req.object).toBeDefined();
expect(req.object.get('email')).toEqual('test@parse.com');
expect(req.object.get('username')).toEqual('testuser');
});

const user = new Parse.User();
user.setUsername('testuser');
user.setPassword('password');
user.set('email', 'test@parse.com');
await user.signUp();

await Parse.User.requestPasswordReset('test@parse.com');
expect(hit).toBe(1);
expect(sendPasswordResetEmailCalled).toBe(true);
done();
});

it('should be able to block password reset request if an error is thrown', async done => {
let hit = 0;
let sendPasswordResetEmailCalled = false;
const emailAdapter = {
sendVerificationEmail: () => Promise.resolve(),
sendPasswordResetEmail: () => {
sendPasswordResetEmailCalled = true;
},
sendMail: () => {},
};

await reconfigureServer({
appName: 'test',
emailAdapter: emailAdapter,
publicServerURL: 'http://localhost:8378/1',
});

Parse.Cloud.beforePasswordResetRequest(req => {
hit++;
if (req.object.get('isBanned')) {
throw new Error('banned account');
}
});

const user = new Parse.User();
user.setUsername('banneduser');
user.setPassword('password');
user.set('email', 'banned@parse.com');
await user.signUp();
await user.save({ isBanned: true });

try {
await Parse.User.requestPasswordReset('banned@parse.com');
throw new Error('should not have sent password reset email.');
} catch (e) {
expect(e.message).toBe('banned account');
}
expect(hit).toBe(1);
expect(sendPasswordResetEmailCalled).toBe(false);
done();
});

it('should be able to block password reset request if an error is thrown even if the user has an attached file', async done => {
let hit = 0;
let sendPasswordResetEmailCalled = false;
const emailAdapter = {
sendVerificationEmail: () => Promise.resolve(),
sendPasswordResetEmail: () => {
sendPasswordResetEmailCalled = true;
},
sendMail: () => {},
};

await reconfigureServer({
appName: 'test',
emailAdapter: emailAdapter,
publicServerURL: 'http://localhost:8378/1',
});

Parse.Cloud.beforePasswordResetRequest(req => {
hit++;
if (req.object.get('isBanned')) {
throw new Error('banned account');
}
});

const user = new Parse.User();
user.setUsername('banneduser2');
user.setPassword('password');
user.set('email', 'banned2@parse.com');
await user.signUp();
const base64 = 'V29ya2luZyBhdCBQYXJzZSBpcyBncmVhdCE=';
const file = new Parse.File('myfile.txt', { base64 });
await file.save();
await user.save({ isBanned: true, file });

try {
await Parse.User.requestPasswordReset('banned2@parse.com');
throw new Error('should not have sent password reset email.');
} catch (e) {
expect(e.message).toBe('banned account');
}
expect(hit).toBe(1);
expect(sendPasswordResetEmailCalled).toBe(false);
done();
});

it('should not run beforePasswordResetRequest if email does not exist', async done => {
let hit = 0;
const emailAdapter = {
sendVerificationEmail: () => Promise.resolve(),
sendPasswordResetEmail: () => {},
sendMail: () => {},
};

await reconfigureServer({
emailAdapter: emailAdapter,
publicServerURL: 'http://localhost:8378/1',
});

Parse.Cloud.beforePasswordResetRequest(req => {
hit++;
});

try {
await Parse.User.requestPasswordReset('nonexistent@parse.com');
} catch (e) {
// May or may not throw depending on passwordPolicy.resetPasswordSuccessOnInvalidEmail
}
expect(hit).toBe(0);
done();
});

it('should have expected data in request in beforePasswordResetRequest', async done => {
const emailAdapter = {
sendVerificationEmail: () => Promise.resolve(),
sendPasswordResetEmail: () => {},
sendMail: () => {},
};

await reconfigureServer({
appName: 'test',
emailAdapter: emailAdapter,
publicServerURL: 'http://localhost:8378/1',
});

Parse.Cloud.beforePasswordResetRequest(req => {
expect(req.object).toBeDefined();
expect(req.object.get('email')).toBeDefined();
expect(req.headers).toBeDefined();
expect(req.ip).toBeDefined();
expect(req.installationId).toBeDefined();
expect(req.context).toBeDefined();
expect(req.config).toBeDefined();
});

const user = new Parse.User();
user.setUsername('testuser2');
user.setPassword('password');
user.set('email', 'test2@parse.com');
await user.signUp();
await Parse.User.requestPasswordReset('test2@parse.com');
done();
});

it('should validate that only _User class is allowed for beforePasswordResetRequest', () => {
expect(() => {
Parse.Cloud.beforePasswordResetRequest('SomeClass', () => { });
}).toThrow('Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers');
expect(() => {
Parse.Cloud.beforePasswordResetRequest(() => { });
}).not.toThrow();
expect(() => {
Parse.Cloud.beforePasswordResetRequest('_User', () => { });
}).not.toThrow();
expect(() => {
Parse.Cloud.beforePasswordResetRequest(Parse.User, () => { });
}).not.toThrow();
});
});
Comment on lines +4660 to +4856
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Refactor tests to use async/await instead of callback pattern.

The test suite provides excellent coverage of the beforePasswordResetRequest hook functionality. However, tests should use async/await with promise-based patterns rather than callback patterns with done(), which is the preferred pattern for new tests in this repository.

Based on learnings.

Apply this refactor pattern to tests 1-5 (test 6 is already correct):

Example refactor for test 1:

-  it('should run beforePasswordResetRequest with valid user', async done => {
+  it('should run beforePasswordResetRequest with valid user', async () => {
     let hit = 0;
     let sendPasswordResetEmailCalled = false;
     const emailAdapter = {
       sendVerificationEmail: () => Promise.resolve(),
       sendPasswordResetEmail: () => {
         sendPasswordResetEmailCalled = true;
       },
       sendMail: () => {},
     };
 
     await reconfigureServer({
       appName: 'test',
       emailAdapter: emailAdapter,
       publicServerURL: 'http://localhost:8378/1',
     });
 
     Parse.Cloud.beforePasswordResetRequest(req => {
       hit++;
       expect(req.object).toBeDefined();
       expect(req.object.get('email')).toEqual('test@parse.com');
       expect(req.object.get('username')).toEqual('testuser');
     });
 
     const user = new Parse.User();
     user.setUsername('testuser');
     user.setPassword('password');
     user.set('email', 'test@parse.com');
     await user.signUp();
 
     await Parse.User.requestPasswordReset('test@parse.com');
     expect(hit).toBe(1);
     expect(sendPasswordResetEmailCalled).toBe(true);
-    done();
   });

Apply the same pattern to tests 2-5 by removing async done =>, removing the done() calls at the end, and ensuring all async operations use await.

🤖 Prompt for AI Agents
In spec/CloudCode.spec.js around lines 4660 to 4856, tests 1–5 for
beforePasswordResetRequest currently use the callback-style "async done =>" and
explicit done() calls; refactor each of those five tests to use promise-style
async/await by removing the done parameter and all done() invocations, ensure
every asynchronous call is awaited (e.g., reconfigureServer, user.signUp,
user.save, Parse.User.requestPasswordReset), and remove any try/catch that only
exists to call done(); keep existing assertions and control-flow (throwing on
unexpected success) but rely on async/await to surface failures.

45 changes: 42 additions & 3 deletions src/Routers/UsersRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Types as TriggerTypes,
getRequestObject,
resolveError,
inflate,
} from '../triggers';
import { promiseEnsureIdempotency } from '../middlewares';
import RestWrite from '../RestWrite';
Expand Down Expand Up @@ -444,21 +445,59 @@ export class UsersRouter extends ClassesRouter {
if (!email && !token) {
throw new Parse.Error(Parse.Error.EMAIL_MISSING, 'you must provide an email');
}

let userResults = null;
let userData = null;

// We can find the user using token
if (token) {
const results = await req.config.database.find('_User', {
userResults = await req.config.database.find('_User', {
_perishable_token: token,
_perishable_token_expires_at: { $lt: Parse._encode(new Date()) },
});
if (results && results[0] && results[0].email) {
email = results[0].email;
if (userResults && userResults.length > 0) {
userData = userResults[0];
if (userData.email) {
email = userData.email;
}
}
// Or using email if no token provided
} else if (typeof email === 'string') {
userResults = await req.config.database.find(
'_User',
{ $or: [{ email }, { username: email, email: { $exists: false } }] },
{ limit: 1 },
Auth.maintenance(req.config)
);
Comment on lines +454 to +471
Copy link

@coderabbitai coderabbitai bot Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Token lookup uses inverted expiry comparator and lacks Auth.maintenance/limit.

  • _perishable_token_expires_at should be greater than “now” for a valid token.
  • Use Auth.maintenance like the email path for consistency.
  • Limit results to 1.

Apply:

-      userResults = await req.config.database.find('_User', {
-        _perishable_token: token,
-        _perishable_token_expires_at: { $lt: Parse._encode(new Date()) },
-      });
+      userResults = await req.config.database.find(
+        '_User',
+        {
+          _perishable_token: token,
+          _perishable_token_expires_at: { $gt: Parse._encode(new Date()) },
+        },
+        { limit: 1 },
+        Auth.maintenance(req.config)
+      );
🤖 Prompt for AI Agents
In src/Routers/UsersRouter.js around lines 454 to 471, the token lookup query
uses the wrong expiry comparator and is missing the same options as the email
path; change the _perishable_token_expires_at check to $gt Parse._encode(new
Date()) so only unexpired tokens match, and call req.config.database.find with
the same options as the email branch: pass { limit: 1 } and
Auth.maintenance(req.config) as the query options so the lookup is limited to
one result and runs under maintenance context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtrezza an opininion since this code already exists before my modifcations ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

if (userResults && userResults.length > 0) {
userData = userResults[0];
}
}

if (typeof email !== 'string') {
throw new Parse.Error(
Parse.Error.INVALID_EMAIL_ADDRESS,
'you must provide a valid email string'
);
}

if (userData) {
this._sanitizeAuthData(userData);
// Useful to get User attached files in the trigger (photo picture for example)
await req.config.filesController.expandFilesInObject(req.config, userData);

const user = inflate('_User', userData);

await maybeRunTrigger(
TriggerTypes.beforePasswordResetRequest,
req.auth,
user,
null,
req.config,
req.info.context
);
}

const userController = req.config.userController;
try {
await userController.sendPasswordResetEmail(email);
Expand Down
43 changes: 43 additions & 0 deletions src/cloud-code/Parse.Cloud.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,49 @@ ParseCloud.afterLogout = function (handler) {
triggers.addTrigger(triggers.Types.afterLogout, className, handler, Parse.applicationId);
};

/**
*
* Registers the before password reset request function.
*
* **Available in Cloud Code only.**
*
* This function provides control in validating a password reset request
* before the reset email is sent. It is triggered after the user is found
* by email, but before the reset token is generated and the email is sent.
*
* ```
* Parse.Cloud.beforePasswordResetRequest((request) => {
* // Validate email or user properties
* if (!request.object.get('emailVerified')) {
* throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, 'Email not verified');
* }
* })
*
* ```
*
* @method beforePasswordResetRequest
* @name Parse.Cloud.beforePasswordResetRequest
* @param {Function} func The function to run before a password reset request. This function can be async and should take one parameter a {@link Parse.Cloud.TriggerRequest};
*/
ParseCloud.beforePasswordResetRequest = function (handler, validationHandler) {
let className = '_User';
if (typeof handler === 'string' || isParseObjectConstructor(handler)) {
// validation will occur downstream, this is to maintain internal
// code consistency with the other hook types.
className = triggers.getClassName(handler);
handler = arguments[1];
validationHandler = arguments.length >= 2 ? arguments[2] : null;
}
triggers.addTrigger(triggers.Types.beforePasswordResetRequest, className, handler, Parse.applicationId);
if (validationHandler && validationHandler.rateLimit) {
addRateLimit(
{ requestPath: `/requestPasswordReset`, requestMethods: 'POST', ...validationHandler.rateLimit },
Parse.applicationId,
true
);
}
};

/**
* Registers an after save function.
*
Expand Down
6 changes: 4 additions & 2 deletions src/triggers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const Types = {
beforeLogin: 'beforeLogin',
afterLogin: 'afterLogin',
afterLogout: 'afterLogout',
beforePasswordResetRequest: 'beforePasswordResetRequest',
beforeSave: 'beforeSave',
afterSave: 'afterSave',
beforeDelete: 'beforeDelete',
Expand Down Expand Up @@ -58,10 +59,10 @@ function validateClassNameForTriggers(className, type) {
// TODO: Allow proper documented way of using nested increment ops
throw 'Only afterSave is allowed on _PushStatus';
}
if ((type === Types.beforeLogin || type === Types.afterLogin) && className !== '_User') {
if ((type === Types.beforeLogin || type === Types.afterLogin || type === Types.beforePasswordResetRequest) && className !== '_User') {
// TODO: check if upstream code will handle `Error` instance rather
// than this anti-pattern of throwing strings
throw 'Only the _User class is allowed for the beforeLogin and afterLogin triggers';
throw 'Only the _User class is allowed for the beforeLogin, afterLogin, and beforePasswordResetRequest triggers';
}
if (type === Types.afterLogout && className !== '_Session') {
// TODO: check if upstream code will handle `Error` instance rather
Expand Down Expand Up @@ -287,6 +288,7 @@ export function getRequestObject(
triggerType === Types.afterDelete ||
triggerType === Types.beforeLogin ||
triggerType === Types.afterLogin ||
triggerType === Types.beforePasswordResetRequest ||
triggerType === Types.afterFind
) {
// Set a copy of the context on the request object.
Expand Down
Loading