chore(sync): merge upstream documenso/main (v2.18.0..c81bc72c4) - #14
Conversation
Selecting password auth failed with a generic "Unauthorized" for users who signed up via OAuth or passkey, with no way to set one. Detect the missing password and email the existing reset link from the signing dialog and security settings. Require a 2FA code and rate limit update-password.
22 upstream commits since v2.18.0: inbox filters, signing reason override env, template naming options, passwordless action auth fix (documenso#3358), bulk download dialog fix, base path email asset fix, and chores. Conflicts resolved: signing/index.ts takes the upstream NEXT_PRIVATE_SIGNING_REASON() (branding default handled by the branding patch), openpage-api cors.ts takes the upstream simplified cors (caller signatures already match). # Conflicts: # apps/openpage-api/lib/cors.ts # packages/signing/index.ts
|
Important Review skippedToo many files! This PR contains 129 files, which is 29 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (129)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements, including support for a customizable PDF signing reason, updated API documentation for CSC signing errors, and a migration to nuqs for search query state management across various routes. It also adds 2FA verification to password updates and introduces a password setup flow for passwordless users. Feedback on these changes highlights a critical error-handling issue where a notification failure after a successful password update transaction could cause a mismatch, a UI layout bug with nested PinInputGroup elements in the 2FA dialog, and a robustness improvement for handling empty member names in the templates filter.
| await jobsClient.triggerJob({ | ||
| name: 'send.password.reset.success.email', | ||
| payload: { | ||
| userId, | ||
| source: 'UPDATE', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The jobsClient.triggerJob call is executed after the database transaction has successfully committed. If the job queue or Redis is temporarily unavailable, this call will throw an error, causing the entire updatePassword operation to fail with a 500 error. However, because the database transaction already succeeded, the user's password will have been updated in the database. This creates a critical mismatch where the user receives a failure message but their password was actually changed. Wrapping this call in a try/catch block ensures that a notification failure does not disrupt a successful password update.
try {
await jobsClient.triggerJob({
name: 'send.password.reset.success.email',
payload: {
userId,
source: 'UPDATE',
},
});
} catch (err) {
console.error('Failed to trigger password update notification email', err);
}| {Array(6) | ||
| .fill(null) | ||
| .map((_, i) => ( | ||
| <PinInputGroup key={i}> | ||
| <PinInputSlot index={i} /> | ||
| </PinInputGroup> | ||
| ))} |
There was a problem hiding this comment.
Wrapping each PinInputSlot in its own PinInputGroup inside the loop will render six separate groups. In standard PIN/OTP input components, PinInputGroup acts as a styled container that manages borders and rounded corners for its child slots. Creating a separate group for each slot will break the visual layout (e.g., rendering individual rounded boxes instead of a single cohesive input field). Wrapping the loop inside a single PinInputGroup will fix the layout.
| {Array(6) | |
| .fill(null) | |
| .map((_, i) => ( | |
| <PinInputGroup key={i}> | |
| <PinInputSlot index={i} /> | |
| </PinInputGroup> | |
| ))} | |
| <PinInputGroup> | |
| {Array(6) | |
| .fill(null) | |
| .map((_, i) => ( | |
| <PinInputSlot key={i} index={i} /> | |
| ))} | |
| </PinInputGroup> |
| }); | ||
|
|
||
| const options = (data ?? []).map((member) => ({ | ||
| label: member.name ?? member.email, |
There was a problem hiding this comment.
Using the nullish coalescing operator (??) on member.name will only fall back to member.email if member.name is strictly null or undefined. If a member's name is saved as an empty string (""), it will be used as the label, resulting in an empty option in the filter dropdown. Using the logical OR operator (||) and trimming the name is more robust and ensures a proper fallback.
| label: member.name ?? member.email, | |
| label: member.name?.trim() || member.email, |
9 participants
What
First upstream sync audit + merge of the 22 commits documenso/documenso landed on main since our fork base tag v2.18.0 (no v2.19.0 release exists yet, so the release-based sync script is a no-op today - this merges main directly, matching the fork's precedent commit 8606464).
Highlights: inbox filters, signing reason override env (
NEXT_PRIVATE_SIGNING_REASON), template naming options, passwordless action auth fix (documenso#3358), bulk download dialog fix, base-path email asset fix.Conflict resolution (2 files)
packages/signing/index.ts: took upstream'sNEXT_PRIVATE_SIGNING_REASON(); our hardcoded branding moves to the prod env (NEXT_PRIVATE_SIGNING_REASON="Signed by Crove Sign") applied during deploy - zero code divergence.apps/openpage-api/lib/cors.ts: took upstream's simplified cors; caller call-shapes are identical on both sides so no caller churn.The branding patch (scripts/patch-crove-branding.mjs) ran post-merge: no source changes needed (favicon already matched).
Verification
Deploy note
Production env addition required at deploy time:
NEXT_PRIVATE_SIGNING_REASON="Signed by Crove Sign"(upstream default is the Documenso string).