-
Notifications
You must be signed in to change notification settings - Fork 566
feat: handle undefine username/password for SMTP #2011
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR downgrades the MongoDB driver from version 6.20.0 to 5.9.2, improves SMTP authentication configuration to handle optional credentials, refactors signature management to use Parse Cloud Functions, and fixes error messages for consistency.
- Downgraded MongoDB driver to version 5.9.2 to address compatibility issues
- Enhanced SMTP configuration to conditionally add authentication only when both username and password are provided
- Refactored signature operations from direct Parse SDK queries to server-side Cloud Functions for better security and maintainability
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/OpenSignServer/package.json | Downgraded mongodb dependency from 6.20.0 to 5.9.2 |
| apps/OpenSignServer/package-lock.json | Updated lock file with mongodb 5.9.2 and new dependencies (ip-address, smart-buffer, socks) |
| apps/OpenSignServer/index.js | Modified SMTP configuration to conditionally add auth only when credentials exist |
| apps/OpenSignServer/cloud/parsefunction/sendMailv3.js | Applied same conditional SMTP auth logic and improved error messages |
| apps/OpenSignServer/cloud/parsefunction/saveSignature.js | New Cloud Function to save/update signatures with authorization checks |
| apps/OpenSignServer/cloud/parsefunction/manageSign.js | New Cloud Function for managing signatures with different field handling |
| apps/OpenSignServer/cloud/parsefunction/getSignature.js | New Cloud Function to retrieve user signatures |
| apps/OpenSignServer/cloud/main.js | Registered new Cloud Functions |
| apps/OpenSign/src/utils/widgetUtils.js | Refactored to use savesignature Cloud Function |
| apps/OpenSign/src/pages/Managesign.jsx | Refactored to use managesign and getdefaultsignature Cloud Functions |
| apps/OpenSign/src/constant/Utils.js | Fixed comment typo and refactored to use getdefaultsignature Cloud Function |
Files not reviewed (1)
- apps/OpenSignServer/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Missing userId parameter.'); | ||
| } | ||
| if (userId !== request.user?.id) { | ||
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message 'Cannot save signature for the current user' is misleading. When userId !== request.user?.id, the user is attempting to save a signature for a different user (not the current user), which should be prevented. The message should be 'Cannot save signature for another user.' or 'You can only save signatures for yourself.'
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); | |
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'You can only save signatures for yourself.'); |
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Missing userId parameter.'); | ||
| } | ||
| if (userId !== request.user?.id) { | ||
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message 'Cannot save signature for the current user' is misleading. When userId !== request.user?.id, the user is attempting to save a signature for a different user (not the current user), which should be prevented. The message should be 'Cannot save signature for another user.' or 'You can only save signatures for yourself.'
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); | |
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'You can only save signatures for yourself.'); |
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Missing userId parameter.'); | ||
| } | ||
| if (userId !== request.user?.id) { | ||
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message 'Cannot save signature for the current user' is incorrect for a get operation. This function retrieves signatures, not saves them. The message should be 'Cannot retrieve signature for another user.' or 'You can only retrieve your own signatures.'
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot save signature for the current user.'); | |
| throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot retrieve signature for another user.'); |
| if (userPtr) { | ||
| signatureCls.set('UserId', userPtr); | ||
| } |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition 'if (userPtr)' is unnecessary because userPtr is defined unconditionally on line 10. Since userId is already validated to be non-null on lines 4-6, userPtr will always exist. This check should be removed and 'UserId' should always be set.
| if (userPtr) { | |
| signatureCls.set('UserId', userPtr); | |
| } | |
| signatureCls.set('UserId', userPtr); |
| if (userPtr) { | ||
| signatureCls.set('UserId', userPtr); | ||
| } |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition 'if (userPtr)' is unnecessary because userPtr is defined unconditionally on line 10. Since userId is already validated to be non-null on lines 4-6, userPtr will always exist. This check should be removed and 'UserId' should always be set.
| if (userPtr) { | |
| signatureCls.set('UserId', userPtr); | |
| } | |
| signatureCls.set('UserId', userPtr); |
No description provided.