feat: add document auth #1029
feat: add document auth #1029
Conversation
|
Important Auto Review SkippedAuto reviews are limited to the following labels: coderabbit. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe updates span across multiple components and libraries, primarily focusing on enhancing document and recipient authentication, improving error handling, and refining the UI/UX for document management processes. New functionalities include settings configuration for documents, advanced authentication options, and streamlined document flow management. These changes aim to provide a more secure, flexible, and user-friendly experience for document creation, editing, and signing processes. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
github-actions
bot
added
apps: web
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 16
Configuration used: CodeRabbit UI
Files selected for processing (50)
- apps/marketing/src/app/(marketing)/singleplayer/client.tsx (1 hunks)
- apps/web/src/app/(dashboard)/documents/[id]/edit-document.tsx (11 hunks)
- apps/web/src/app/(signing)/sign/[token]/complete/page.tsx (4 hunks)
- apps/web/src/app/(signing)/sign/[token]/date-field.tsx (2 hunks)
- apps/web/src/app/(signing)/sign/[token]/document-action-auth-dialog.tsx (1 hunks)
- apps/web/src/app/(signing)/sign/[token]/document-auth-provider.tsx (1 hunks)
- apps/web/src/app/(signing)/sign/[token]/email-field.tsx (2 hunks)
- apps/web/src/app/(signing)/sign/[token]/form.tsx (4 hunks)
- apps/web/src/app/(signing)/sign/[token]/name-field.tsx (6 hunks)
- apps/web/src/app/(signing)/sign/[token]/page.tsx (3 hunks)
- apps/web/src/app/(signing)/sign/[token]/sign-dialog.tsx (2 hunks)
- apps/web/src/app/(signing)/sign/[token]/signature-field.tsx (8 hunks)
- apps/web/src/app/(signing)/sign/[token]/signing-auth-page.tsx (1 hunks)
- apps/web/src/app/(signing)/sign/[token]/signing-field-container.tsx (3 hunks)
- apps/web/src/app/(signing)/sign/[token]/signing-page-view.tsx (1 hunks)
- apps/web/src/app/(signing)/sign/[token]/text-field.tsx (7 hunks)
- apps/web/src/components/document/document-history-sheet.tsx (4 hunks)
- packages/lib/constants/document-auth.ts (1 hunks)
- packages/lib/errors/app-error.ts (1 hunks)
- packages/lib/server-only/document/complete-document-with-token.ts (3 hunks)
- packages/lib/server-only/document/get-document-by-token.ts (4 hunks)
- packages/lib/server-only/document/is-recipient-authorized.ts (1 hunks)
- packages/lib/server-only/document/update-document-settings.ts (1 hunks)
- packages/lib/server-only/document/viewed-document.ts (2 hunks)
- packages/lib/server-only/field/sign-field-with-token.ts (4 hunks)
- packages/lib/server-only/field/update-field.ts (4 hunks)
- packages/lib/server-only/recipient/set-recipients-for-document.ts (6 hunks)
- packages/lib/types/document-audit-logs.ts (10 hunks)
- packages/lib/types/document-auth.ts (1 hunks)
- packages/lib/utils/document-audit-logs.ts (5 hunks)
- packages/lib/utils/document-auth.ts (1 hunks)
- packages/prisma/migrations/20240311113243_add_document_auth/migration.sql (1 hunks)
- packages/prisma/schema.prisma (2 hunks)
- packages/trpc/server/document-router/router.ts (4 hunks)
- packages/trpc/server/document-router/schema.ts (3 hunks)
- packages/trpc/server/field-router/router.ts (2 hunks)
- packages/trpc/server/field-router/schema.ts (2 hunks)
- packages/trpc/server/recipient-router/router.ts (2 hunks)
- packages/trpc/server/recipient-router/schema.ts (3 hunks)
- packages/trpc/server/trpc.ts (1 hunks)
- packages/ui/components/animate/animate-generic-fade-in-out.tsx (1 hunks)
- packages/ui/components/document/document-download-button.tsx (1 hunks)
- packages/ui/primitives/checkbox.tsx (1 hunks)
- packages/ui/primitives/document-flow/add-settings.tsx (1 hunks)
- packages/ui/primitives/document-flow/add-settings.types.ts (1 hunks)
- packages/ui/primitives/document-flow/add-signers.tsx (8 hunks)
- packages/ui/primitives/document-flow/add-signers.types.ts (2 hunks)
- packages/ui/primitives/document-flow/add-subject.tsx (4 hunks)
- packages/ui/primitives/document-flow/add-subject.types.ts (1 hunks)
- packages/ui/primitives/input.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/trpc/server/trpc.ts
Additional comments: 123
packages/prisma/migrations/20240311113243_add_document_auth/migration.sql (2)
- 2-2: Ensure that the
authOptionscolumn in the "Document" table is indexed if it will be queried frequently. This can help improve query performance.- 5-5: Similar to the "Document" table, consider indexing the
authOptionscolumn in the "Recipient" table if it's expected to be a common query parameter.packages/ui/primitives/document-flow/add-subject.types.ts (1)
- 1-9: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-10]
The schema definition for
ZAddSubjectFormSchemais concise and correctly uses Zod for validation. However, ensure that the removal of default values fortimezone,dateFormat, andredirectUrlis intentional and does not impact other parts of the application expecting these fields.Verification successful
The verification process has revealed that
timezone,dateFormat, andredirectUrlare indeed used across various parts of the application, with explicit default values set in some contexts. This suggests that the removal or modification of these fields or their default values in one part of the application should be carefully considered to ensure consistency and avoid unintended side effects on the application's functionality. It is crucial to verify that any changes made align with the overall application design and do not negatively impact other components that rely on these fields.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if other parts of the application are impacted by the removal of default values. rg 'timezone|dateFormat|redirectUrl' --type tsLength of output: 10143
packages/ui/primitives/input.tsx (1)
- 13-13: The updated CSS class composition for the input element, including the transition effect for the
aria-invalidstate, enhances the user interface by providing visual feedback on input validation. Ensure that the transition effect is tested across different browsers for consistency.packages/lib/constants/document-auth.ts (1)
- 17-31: The definition of
DOCUMENT_AUTH_TYPESand the introduction of theisAuthRedirectRequiredfield align with the PR objectives to enhance document authentication. Ensure that theisAuthRedirectRequiredfield is appropriately utilized in the authentication flow to redirect users when necessary.packages/ui/primitives/document-flow/add-signers.types.ts (1)
- 17-19: The inclusion of the
actionAuthfield with optional validation usingZRecipientActionAuthTypesSchemainZAddSignersFormSchemais a significant enhancement that aligns with the PR objectives. Ensure that the form correctly handles the optional nature ofactionAuthand provides appropriate feedback to the user.packages/ui/primitives/checkbox.tsx (1)
- 19-19: The addition of the
bg-backgroundclass to theCheckboxcomponent enhances the visual consistency of the checkbox with the application's design. Ensure that the change is tested across different themes (if applicable) to maintain visual consistency.packages/ui/primitives/document-flow/add-settings.types.ts (1)
- 22-40: The definition of
ZAddSettingsFormSchema, including the global access and action authentication settings, as well as the optional fields with default values and validation, is a comprehensive enhancement that aligns with the PR objectives. Ensure that the form correctly handles the optional nature of these fields and provides appropriate feedback to the user.packages/ui/components/document/document-download-button.tsx (1)
- 63-63: The conditional rendering logic to show the download icon only when not in a loading state is correctly implemented and enhances the user experience by providing visual feedback during the download operation.
packages/trpc/server/field-router/schema.ts (1)
- 49-49: The addition of the
authOptionsfield toZSignFieldWithTokenMutationSchemais correctly implemented, enhancing the document authentication capabilities as intended. The use of Zod for schema validation is appropriate and ensures type safety and validation.packages/trpc/server/recipient-router/schema.ts (2)
- 19-19: The addition of the
actionAuthfield toZAddSignersMutationSchemais correctly implemented, enhancing recipient-specific authentication capabilities. The use of Zod for schema validation ensures type safety and validation.- 62-62: The addition of the
authOptionsfield toZCompleteDocumentWithTokenMutationSchemais correctly implemented, further enhancing document authentication capabilities. The optional nature of the field ensures backward compatibility and flexibility.apps/web/src/app/(signing)/sign/[token]/signing-auth-page.tsx (1)
- 1-67: The implementation of the
SigningAuthPageViewcomponent is well-done, correctly integrating with the new authentication features. The use of React hooks, async/await, and error handling patterns is appropriate and enhances the user experience by providing clear instructions and feedback.packages/lib/server-only/document/viewed-document.ts (2)
- 14-14: The addition of the
recipientAccessAuthfield toViewedDocumentOptionsis correctly implemented, enhancing the handling of document viewing with new authentication options. The careful consideration of this field in theviewedDocumentfunction aligns with the objectives of enhancing document authentication.- 60-60: The update to include
recipientAccessAuthin the audit log creation process is a good practice, ensuring that the new authentication options are comprehensively tracked. This enhances the auditability of document access.packages/lib/utils/document-auth.ts (1)
- 12-72: The utility functions
extractDocumentAuthMethods,createDocumentAuthOptions, andcreateRecipientAuthOptionsare well-implemented, providing a clear and type-safe way to handle document and recipient authentication options. The use of Zod for validation enhances the robustness of the authentication feature.packages/lib/server-only/document/is-recipient-authorized.ts (1)
- 10-86: The implementation of the
isRecipientAuthorizedfunction is well-done, correctly determining recipient authorization based on the new authentication options. The use of pattern matching for handling different authentication methods is expressive and enhances the readability and maintainability of the code.packages/trpc/server/recipient-router/router.ts (2)
- 31-31: The addition of
actionAuthto the signer object in theaddSignersmutation is correctly implemented, enhancing the flexibility and security of the document signing process by incorporating recipient-specific authentication options.- 75-81: The addition of
authOptionsto thecompleteDocumentWithTokenfunction and the use ofctx.user?.idfor setting theuserIdare correctly implemented, aligning with the objectives of enhancing document authentication and ensuring the use of the current user's ID when available.packages/lib/server-only/field/update-field.ts (3)
- 4-6: The addition of
DOCUMENT_AUDIT_LOG_TYPEanddiffFieldChangesimports indicates an enhancement in audit logging capabilities for field updates. Ensure these utilities are properly implemented and tested.- 37-37: Using
findFirstOrThrowinstead of a direct update ensures that the field exists before attempting an update, enhancing error handling. This change is a good practice for data integrity and error management.- 80-97: The update to
createDocumentAuditLogDatacall with additional parameters and changes in the data object structure enhances the detail and accuracy of audit logs. Ensure that the new parameters and data structure are correctly documented and validated.Consider adding validation for the new parameters to ensure they meet expected formats and values before proceeding with the audit log creation.
apps/web/src/app/(signing)/sign/[token]/email-field.tsx (2)
- 9-10: The addition of
AppErrorandAppErrorCodeimports for improved error handling is a positive change. It aligns with best practices for user feedback and error management in UI components.- 55-59: Adding error handling for
AppErrorCode.UNAUTHORIZEDis essential for a better user experience, especially in scenarios requiring authentication. This change ensures that unauthorized actions are caught and handled gracefully.packages/trpc/server/field-router/router.ts (2)
- 3-3: The addition of
AppErrorimport for enhanced error handling in thefieldRouteris a good practice. It aligns with the application's error handling strategy and improves maintainability.- 89-89: Replacing the error handling logic to throw
AppErrorconverted toTRPCErroris a significant improvement. It ensures consistency in error handling across the application and enhances the user experience by providing more descriptive error messages.packages/lib/server-only/document/get-document-by-token.ts (2)
- 4-6: The addition of imports for error handling and type definitions is a positive change, enhancing the file's functionality and maintainability. Ensure these new utilities and types are correctly implemented and documented.
- 69-90: Updating
getDocumentAndSenderByTokenandgetDocumentAndRecipientByTokenfunctions to handle access authorization based on the recipient enhances the application's security model. This change ensures that document access is properly authorized and managed. However, ensure that error handling for unauthorized access is user-friendly and informative.Improve error messages for unauthorized access to provide users with clear guidance on how to obtain access or whom to contact for assistance.
apps/web/src/app/(signing)/sign/[token]/sign-dialog.tsx (1)
- 15-15: The addition of
useRequiredDocumentAuthContextto manage authentication procedures before opening the dialog is a good practice. It ensures that the dialog opens conditionally based on authentication requirements, enhancing security and user experience.apps/web/src/app/(signing)/sign/[token]/date-field.tsx (2)
- 14-15: The addition of
AppErrorandAppErrorCodeimports for improved error handling is a positive change. It aligns with best practices for user feedback and error management in UI components.- 69-73: Adding error handling for
AppErrorCode.UNAUTHORIZEDis essential for a better user experience, especially in scenarios requiring authentication. This change ensures that unauthorized actions are caught and handled gracefully.apps/web/src/app/(signing)/sign/[token]/signing-field-container.tsx (2)
- 5-5: The addition of
TRecipientActionAuthimport indicates an enhancement in handling authentication for signing fields. Ensure this type is properly used and integrated within the component's logic.- 10-10: Incorporating
useRequiredDocumentAuthContextfor managing authentication procedures is a good practice. It ensures that the component interacts correctly with the application's authentication context, enhancing security and user experience.apps/web/src/app/(signing)/sign/[token]/page.tsx (7)
- 7-7: The import of
isRecipientAuthorizedindicates the addition of authorization logic for recipients. Ensure that this function is thoroughly tested, especially for edge cases and error handling.- 14-14: The replacement of
truncateTitlewithextractDocumentAuthMethodssuggests a shift towards more complex document authentication logic. It's crucial to verify that this change does not impact other areas of the application wheretruncateTitlewas used.Verification successful
The search results indicate that
truncateTitleis still being used in various parts of the application, including within thesigning-page-view.tsx,sign-dialog.tsx,complete/page.tsx, andtrigger-multiselect-combobox.tsxfiles. This suggests that the replacement oftruncateTitlewithextractDocumentAuthMethodsin the specific context ofsign/[token]/page.tsxdoes not eliminate the use oftruncateTitleelsewhere in the application. Therefore, the concern that the replacement might impact other areas wheretruncateTitlewas used is unfounded, astruncateTitlecontinues to be utilized in other components.* 17-21: Introduction of `DocumentAuthProvider` and `SigningPageView` components. This change centralizes the authentication logic and UI rendering for signing pages. Ensure that these components are modular and reusable for future authentication methods. * 54-57: The use of `extractDocumentAuthMethods` to derive recipient access authentication methods is a significant change. It's important to ensure that the logic correctly handles all possible authentication configurations. * 59-64: The logic to check document access validity using `isRecipientAuthorized` is crucial for security. Ensure comprehensive testing around this logic to prevent unauthorized access. * 70-74: The call to `viewedDocument` upon successful document access validation is a good practice for audit logging. Verify that error handling is in place for any potential failures in this call. * 122-123: The use of `DocumentAuthProvider` to wrap `SigningPageView` indicates a structured approach to handling document and recipient authentication. This pattern should enhance the maintainability of the authentication logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of `truncateTitle` to ensure they've been properly replaced or removed. rg --type tsx --type ts 'truncateTitle'Length of output: 67
Script:
#!/bin/bash # Corrected script to search for usages of `truncateTitle` without specifying file types, to avoid the previous error. rg 'truncateTitle'Length of output: 1097
packages/lib/types/document-auth.ts (1)
- 1-121: The introduction of various Zod schemas for document and recipient authentication types is a robust approach to ensuring data integrity and validation. It's important to ensure that these schemas are comprehensive and align with the application's authentication requirements.
packages/lib/errors/app-error.ts (1)
- 140-148: The addition of a try-catch block in the
parseFromJSONStringmethod improves the robustness of JSON parsing error handling. This change helps prevent potential crashes due to malformed JSON strings. Ensure that this error handling strategy is consistent with other parts of the application.packages/trpc/server/document-router/schema.ts (2)
- 44-66: The introduction of
ZSetSettingsForDocumentMutationSchemais a significant addition that allows for the validation of document settings updates. Ensure that this schema accurately reflects all possible settings that can be updated and consider future extensibility for additional settings.- 119-120: Making
timezoneanddateFormatoptional inZSendDocumentMutationSchemareflects a simplification of the document sending process. Verify that this change aligns with the application's requirements and does not impact user experience negatively.packages/lib/server-only/document/complete-document-with-token.ts (2)
- 82-97: The addition of authorization checks using
isRecipientAuthorizedbefore allowing a document to be completed is crucial for security. Ensure that this logic correctly handles all authentication types and properly denies access when necessary.- 99-127: The transactional update logic for marking a recipient as signed and creating an audit log entry is well-structured. Ensure that error handling is in place for each step of the transaction to maintain data integrity in case of failures.
packages/ui/primitives/document-flow/add-subject.tsx (3)
- 4-4: The change from
Controller, useFormto onlyuseFormindicates a simplification in form handling. Ensure that this change does not remove any required functionality or negatively impact form behavior.- 44-44: The removal of
touchedFieldsfrom the form state indicates a simplification in form state management. Verify that this change does not affect form validation or user feedback mechanisms negatively.- 127-132: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [58-161]
The significant refactoring of the
AddSubjectFormPartialcomponent to focus on essential fields and remove advanced options like date format, time zone, and redirect URL simplifies the user interface. Ensure that this simplification aligns with the application's requirements and does not remove any critical functionality.packages/lib/server-only/document/update-document-settings.ts (3)
- 33-35: The validation to ensure that there is data to update is a good practice. It prevents unnecessary database operations and ensures that the API call has a clear intent.
- 83-88: Restricting title updates to documents in the DRAFT status is a critical business rule. Ensure that this rule is clearly communicated to users to avoid confusion.
- 105-132: The creation of audit logs for changes in global access and action authentication settings is essential for tracking and auditing. Verify that these logs capture all necessary information and are created only when changes occur.
apps/web/src/app/(signing)/sign/[token]/document-auth-provider.tsx (1)
- 58-161: The introduction of
DocumentAuthProvideras a context provider for document authentication is a significant architectural enhancement. It centralizes authentication logic, making it easier to manage and reuse. Ensure that this provider is flexible enough to accommodate future authentication methods and scenarios.apps/web/src/app/(signing)/sign/[token]/text-field.tsx (7)
- 9-10: Imports for
AppError,AppErrorCode, andTRecipientActionAuthhave been added to handle errors and type definitions for recipient action authentication. This aligns with the PR's objective to enhance document authentication mechanisms.- 20-20: The import of
useRequiredDocumentAuthContextis crucial for accessing the document authentication context, enabling the component to execute authentication procedures. This is a necessary addition for supporting the new authentication features.- 33-33: The addition of
executeActionAuthProcedurefrom the document authentication context is essential for handling the authentication process during the signing action. This function likely encapsulates the logic for verifying authentication requirements, which is key to the new authentication enhancements.- 59-65: The
onDialogSignClickfunction, which handles the dialog sign click event, correctly sets the modal visibility to false and invokes theexecuteActionAuthProcedurewith a callback for theonSignfunction. This approach ensures that the signing process is contingent upon successful authentication, aligning with the PR's objectives.- 67-73: The
onPreSignfunction checks if the local text is empty and, if so, displays the custom text modal. This preemptive check is a good practice to ensure that a user cannot proceed with signing without entering the required text, enhancing the user experience and data integrity.- 46-80: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [76-87]
The
onSignfunction has been modified to include an optionalauthOptionsparameter, which is used when signing the field. This modification is crucial for integrating the new authentication mechanism into the signing process, ensuring that the action authentication requirements are met before proceeding.
- 130-136: The
SigningFieldContainercomponent has been updated with new props, includingonPreSign,onSign, andonRemove. These updates are necessary to integrate the new authentication and signing logic into the component, ensuring that the signing process adheres to the enhanced authentication requirements.packages/lib/server-only/field/sign-field-with-token.ts (6)
- 11-13: The addition of imports for
AppError,AppErrorCode, andTRecipientActionAuthis necessary for error handling and type definitions related to recipient action authentication. These changes are consistent with the PR's goal of enhancing document authentication.- 16-17: The import of utility functions
extractDocumentAuthMethodsandisRecipientAuthorizedindicates the introduction of more sophisticated authentication checks. These functions likely play a critical role in determining the appropriate authentication requirements and verifying if a recipient meets those requirements.- 24-25: The addition of
userIdandauthOptionsto theSignFieldWithTokenOptionstype is crucial for supporting the new authentication features. These parameters enable the function to perform recipient-specific authentication checks based on the provided authentication options.- 82-93: The use of
extractDocumentAuthMethodsto derive recipient action authentication and the subsequent check withisRecipientAuthorizedare key enhancements for enforcing authentication requirements. This logic ensures that only authorized recipients can perform actions on the document, aligning with the PR's objectives.- 95-97: The error handling for unauthorized access attempts is appropriately implemented using the
AppErrorclass. This ensures that a clear and consistent error response is provided when authentication requirements are not met, enhancing security and user feedback.- 186-190: The update to
fieldSecuritybased on the derived recipient action authentication is a significant change. It ensures that the security settings for a field are dynamically adjusted based on the recipient's authentication status, further enhancing document security.apps/web/src/app/(signing)/sign/[token]/name-field.tsx (7)
- 9-10: The imports for
AppError,AppErrorCode, andTRecipientActionAuthare necessary for handling errors and type definitions for recipient action authentication. These additions are consistent with the PR's goal of enhancing document authentication mechanisms.- 20-20: The import of
useRequiredDocumentAuthContextis crucial for accessing the document authentication context, enabling the component to execute authentication procedures. This addition supports the new authentication features.- 37-37: The addition of
executeActionAuthProcedurefrom the document authentication context is essential for handling the authentication process during the signing action. This function likely encapsulates the logic for verifying authentication requirements, which is key to the new authentication enhancements.- 66-72: The
onDialogSignClickfunction, which handles the dialog sign click event, correctly sets the modal visibility to false and invokes theexecuteActionAuthProcedurewith a callback for theonSignfunction. This approach ensures that the signing process is contingent upon successful authentication, aligning with the PR's objectives.- 54-60: The
onPreSignfunction checks if the provided full name is empty and, if so, displays the full name modal. This preemptive check is a good practice to ensure that a user cannot proceed with signing without entering the required name, enhancing the user experience and data integrity.- 75-89: The
onSignfunction has been modified to include an optionalauthOptionsparameter, which is used when signing the field. This modification is crucial for integrating the new authentication mechanism into the signing process, ensuring that the action authentication requirements are met before proceeding.- 130-136: The
SigningFieldContainercomponent has been updated with new props, includingonPreSign,onSign, andonRemove. These updates are necessary to integrate the new authentication and signing logic into the component, ensuring that the signing process adheres to the enhanced authentication requirements.apps/web/src/app/(signing)/sign/[token]/complete/page.tsx (2)
- 9-11: The addition of imports for
getServerComponentSessionandisRecipientAuthorizedfunctions is crucial for handling session management and recipient authorization checks. These changes support the PR's objective of enhancing document access control.- 62-71: The implementation of a check for document access authorization based on the recipient's email is a significant enhancement. Redirecting to
SigningAuthPageViewif access is not valid ensures that only authorized recipients can view the completed signing page, aligning with the PR's goals of improving document security.apps/web/src/app/(signing)/sign/[token]/signature-field.tsx (7)
- 9-10: The imports for
AppError,AppErrorCode, andTRecipientActionAuthare necessary for handling errors and type definitions for recipient action authentication. These additions are consistent with the PR's goal of enhancing document authentication mechanisms.- 20-20: The import of
useRequiredDocumentAuthContextis crucial for accessing the document authentication context, enabling the component to execute authentication procedures. This addition supports the new authentication features.- 39-39: The addition of
executeActionAuthProcedurefrom the document authentication context is essential for handling the authentication process during the signing action. This function likely encapsulates the logic for verifying authentication requirements, which is key to the new authentication enhancements.- 82-92: The
onDialogSignClickfunction, which handles the dialog sign click event, correctly sets the modal visibility to false and invokes theexecuteActionAuthProcedurewith a callback for theonSignfunction. This approach ensures that the signing process is contingent upon successful authentication, aligning with the PR's objectives.- 70-76: The
onPreSignfunction checks if the provided signature is empty and, if so, displays the signature modal. This preemptive check is a good practice to ensure that a user cannot proceed with signing without providing a signature, enhancing the user experience and data integrity.- 67-103: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [95-109]
The
onSignfunction has been modified to include an optionalauthOptionsparameter, which is used when signing the field. This modification is crucial for integrating the new authentication mechanism into the signing process, ensuring that the action authentication requirements are met before proceeding.
- 150-156: The
SigningFieldContainercomponent has been updated with new props, includingonPreSign,onSign, andonRemove. These updates are necessary to integrate the new authentication and signing logic into the component, ensuring that the signing process adheres to the enhanced authentication requirements.packages/lib/server-only/recipient/set-recipients-for-document.ts (4)
- 2-5: The addition of imports for
TRecipientActionAuthTypesandZRecipientAuthOptionsSchemais necessary for handling type definitions and schema validation related to recipient action authentication. These changes support the PR's objective of enhancing document authentication mechanisms.- 12-12: The import of
createRecipientAuthOptionsis crucial for generating recipient authentication options based on the providedactionAuthvalues. This function likely plays a key role in configuring recipient-specific authentication settings, aligning with the PR's goals of introducing more granular control over document authentication.- 26-26: The addition of the
actionAuthfield toSetRecipientsForDocumentOptionsis essential for supporting the new recipient-specific action authentication settings. This change enables the backend to process and store recipient-specific authentication requirements, enhancing the flexibility and security of the document authentication process.- 120-127: The handling of
actionAuthin recipient data, including the parsing and creation of recipient authentication options, is a significant enhancement. This logic ensures that recipient-specific action authentication settings are correctly processed and stored, furthering the PR's objectives of introducing more granular authentication controls.apps/web/src/app/(signing)/sign/[token]/form.tsx (5)
- 11-11: The import of
TRecipientActionAuthfrom@documenso/lib/types/document-authis correctly added to support the new authentication features. This type will be used in the authentication procedures within the component.- 23-23: The use of
useRequiredDocumentAuthContextis appropriate here as it provides necessary authentication context for the document being signed. This context is crucial for implementing the new authentication logic.- 40-40: The
executeActionAuthProcedurefunction is retrieved from the document authentication context. This is a good practice as it encapsulates the authentication logic outside of the component, making the component cleaner and more maintainable.- 70-73: The
executeActionAuthProcedurefunction is correctly used to handle the authentication process before completing the document. This ensures that the document can only be completed if the user passes the necessary authentication checks.- 76-80: The
completeDocumentfunction is well-implemented, making use of thecompleteDocumentWithTokenmutation to complete the signing process. It correctly handles theauthOptionsparameter, allowing for flexible authentication mechanisms. However, it's important to ensure that theauthOptionsparameter is properly validated and sanitized before being used to prevent security vulnerabilities.apps/web/src/app/(signing)/sign/[token]/document-action-auth-dialog.tsx (4)
- 1-5: The comment at the beginning of the file provides useful context about the presence of commented-out code related to password authentication. It's good practice to leave such comments for future developers, especially when there are plans to reuse the code for upcoming features like passkeys and 2FA.
- 30-30: The use of
useRequiredDocumentAuthContextis appropriate for accessing the recipient's information within the authentication dialog. This ensures that the dialog can adapt based on the recipient's role and other context-specific details.- 71-73: The handling of loading states with
isSigningOutandisSubmittingis a good practice. It ensures that the user interface reflects the ongoing operations, preventing duplicate actions and improving the user experience.- 140-180: The rendering logic within the
DocumentActionAuthDialogcomponent is correctly implemented, handling different document authentication types and providing a user-friendly interface for reauthentication. The use of thematchfunction fromts-patternfor conditional rendering based ondocumentAuthTypeis a clean and readable approach.However, it's important to ensure that all authentication types are handled correctly and that the UI is accessible and user-friendly.
Verification successful
Given the limitations of the provided context and the nature of the initial script, which was a placeholder for manual verification, it's clear that a more direct approach to verify the handling of all authentication types in the UI is required. To proceed, we need to gather more information about the
DocumentAuthenum and its usage within the component. This will help in verifying whether all authentication types are correctly handled in the UI and that the interface is accessible and user-friendly.
Based on the provided script outputs and the analysis of the
DocumentActionAuthDialogcomponent, it appears that the application is designed to handle different document authentication types in a structured and user-friendly manner. The usage of theDocumentAuthenum within a context provider and the conditional rendering logic in the dialog component for at least two authentication types (ACCOUNTandEXPLICIT_NONE) suggests a thoughtful approach to authentication management. While the script output does not provide a complete list of all possibleDocumentAuthenum values, the evidence suggests that the application is likely handling authentication types correctly. However, without direct access to the enum definition and a full understanding of the application's authentication flow, a definitive conclusion on the exhaustive handling of all types cannot be made.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all authentication types are correctly handled in the UI. # This script is a placeholder for manual verification. echo "Manual verification required: Ensure that all authentication types are correctly handled in the UI and that the interface is accessible and user-friendly."Length of output: 318
Script:
#!/bin/bash # Search for the DocumentAuth enum definition rg "enum DocumentAuth" --type-add 'custom:*.tsx' --type 'custom' # Search for usage of DocumentAuth within the component to verify handling of all types rg "DocumentAuth" --type-add 'custom:*.tsx' --type 'custom' -C 10Length of output: 60048
apps/marketing/src/app/(marketing)/singleplayer/client.tsx (1)
- 164-164: The addition of
authOptionsinitialized tonullis aligned with the PR's objectives. Ensure that downstream components consumingplaceholderRecipientproperly handle theauthOptionsfield, especially itsnullinitialization.Verification successful
The search results indicate that
placeholderRecipientis used in a manner consistent with the addition of theauthOptionsfield, primarily within the context ofclient.tsxand for setting up placeholder recipients inadd-template-placeholder-recipients.tsx. There's no direct evidence ofauthOptionsbeing specifically handled, suggesting that its addition is unlikely to cause immediate issues based on the current usage patterns. However, it's important to remain vigilant for potential future issues as more components interact withplaceholderRecipient.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of placeholderRecipient in the project to ensure proper handling of authOptions. rg "placeholderRecipient" --type tsxLength of output: 64
Script:
#!/bin/bash # Corrected search for usage of placeholderRecipient in the project, including both .ts and .tsx files. rg "placeholderRecipient" --glob "*.ts" --glob "*.tsx"Length of output: 727
apps/web/src/app/(dashboard)/documents/[id]/edit-document.tsx (7)
- 20-21: The addition of
AddSettingsFormPartialand its corresponding typeTAddSettingsFormSchemasupports the new settings-based approach to document editing. Ensure these components are correctly implemented and integrated within the application.- 45-46: The update from
titletosettingsin theEditDocumentSteptype and theEditDocumentStepsarray aligns with the PR's objective to enhance document settings management. Ensure that all references and dependencies are updated accordingly.- 64-65: The transition from
addTitletosetSettingsForDocumentreflects the shift towards a more comprehensive settings management approach. Verify that thesetSettingsForDocumentmutation is correctly implemented on the server side and that it handles all necessary settings updates.Verification successful
The
setSettingsForDocumentmutation is implemented within thedocumentRouterinrouter.ts, as part of the server-side code. This mutation takes input that includesdocumentId,teamId,data, andmeta, and performs two main actions:
- If
meta.timezone,meta.dateFormat, ormeta.redirectUrlare provided, it callsupsertDocumentMetato update these meta settings for the document.- It then calls
updateDocumentSettingsto update other document settings based on the provideddata.This implementation indicates that the mutation is designed to handle a comprehensive set of settings updates for a document, including meta information like timezone, date format, and redirect URL, as well as other unspecified settings contained within
data.Given this information, it can be concluded that the transition from
addTitletosetSettingsForDocumentin the client-side code is supported by a corresponding server-side implementation that is capable of handling a broad range of document settings updates. This aligns with the original review comment's observation about the shift towards a more comprehensive settings management approach.* 109-131: > 📝 **NOTE** > This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [112-139]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of setSettingsForDocument mutation. rg "setSettingsForDocument" --type tsLength of output: 462
Script:
#!/bin/bash # Inspect the implementation of setSettingsForDocument mutation in router.ts fd "router.ts" packages/trpc/server/document-router --exec cat {}Length of output: 10374
The renaming of the form submission handler to
onAddSettingsFormSubmitis appropriate for the new settings-based document editing flow. Ensure that the handler correctly processes and submits the new settings data, including error handling.
- 258-264: The use of
AddSettingsFormPartialfor the settings step is consistent with the PR's objectives. Confirm that the component is properly configured to handle the settings data and integrates well with the document flow.- 120-123: The addition of
globalAccessAuthandglobalActionAuthfields in the settings form submission handler supports the new authentication features. Ensure these fields are correctly processed and stored in the backend.- 139-139: The updated error message in the settings form submission handler reflects the broader scope of settings being updated. Ensure that error handling is comprehensive and user-friendly.
packages/trpc/server/document-router/router.ts (2)
- 15-15: The addition of
updateDocumentSettingsandZSetSettingsForDocumentMutationSchemais necessary for the new document settings functionality. Ensure that these imports are correctly implemented and used securely within thesetSettingsForDocumentprocedure.Also applies to: 31-31
- 159-196: The implementation of the
setSettingsForDocumentprocedure is crucial for updating document settings. Ensure that it correctly processes input data, performs necessary validations, and securely updates document settings. Additionally, verify that error handling is comprehensive and user-friendly.packages/lib/utils/document-audit-logs.ts (3)
- 25-25: The addition of
ZRecipientAuthOptionsSchemaand updates to handleauthOptionsin audit logs support the new authentication features. Ensure thatauthOptionsare properly compared and logged, and verify the correctness and completeness of the implementation.Also applies to: 80-112
- 197-239: The updates to
diffDocumentMetaChangesfor comparing additional fields, such asdateFormat,message,subject,timezone, andredirectUrl, enhance the audit logging capabilities. Ensure that these comparisons are accurate and that changes are correctly logged.- 315-322: The extension of
formatDocumentAuditLogActionto include new document authentication types (DOCUMENT_GLOBAL_AUTH_ACCESS_UPDATEDandDOCUMENT_GLOBAL_AUTH_ACTION_UPDATED) improves audit log readability and tracking. Confirm that these new types are correctly handled and that the descriptions are clear and informative.apps/web/src/components/document/document-history-sheet.tsx (4)
- 10-10: Added import for
DOCUMENT_AUTH_TYPESfrom@documenso/lib/constants/document-auth.This import is necessary for handling the new document audit log types related to global authentication updates.
- 83-87: Modified
formatGenericTextfunction to accept an optionaltextparameter and handle null/undefined cases.This modification improves the function's robustness by ensuring it can gracefully handle null or undefined inputs.
- 227-243: Added handling for
DOCUMENT_AUDIT_LOG_TYPE.DOCUMENT_GLOBAL_AUTH_ACCESS_UPDATEDandDOCUMENT_AUDIT_LOG_TYPE.DOCUMENT_GLOBAL_AUTH_ACTION_UPDATEDin the rendering logic.The addition of these cases ensures that updates to global document authentication settings are properly reflected in the document history.
- 244-244: Updated the rendering logic for
DOCUMENT_AUDIT_LOG_TYPE.DOCUMENT_META_UPDATED.While the summary mentions this update, the provided code does not show any modifications specific to this log type. Assuming this is an oversight in the summary, the existing handling of
DOCUMENT_META_UPDATEDappears to be unchanged and correct.packages/lib/types/document-audit-logs.ts (4)
- 31-32: Added new constants for document audit log types related to global authentication access and action updates.
These new constants are essential for supporting the newly introduced global authentication settings for documents.
- 58-64: Extended schemas for recipient diffs to include access authentication and action authentication.
Extending these schemas is necessary to support the new authentication features at the recipient level, allowing for more granular control.
- 258-272: Updated event schemas for global authentication access and action updates.
These updates to the event schemas are crucial for accurately representing changes to global authentication settings in the audit logs.
- 289-291: Modified event schemas for document opened and recipient completed to include optional access authentication and action authentication fields.
Including these optional fields in the event schemas allows for tracking authentication requirements for document access and recipient actions, enhancing security and audit capabilities.
Also applies to: 299-301
packages/prisma/schema.prisma (1)
- 229-229: Added a new field
authOptionsof typeJsonto both theDocumentandRecipientmodels.The addition of the
authOptionsfield is crucial for supporting the new document and recipient-specific authentication settings, allowing for flexible and granular authentication configurations.Also applies to: 327-327
packages/ui/primitives/document-flow/add-settings.tsx (4)
- 1-1: Using
'use client';at the top of the file is a good practice for Next.js files that are intended to run only on the client side. This ensures that the file's code will not be accidentally executed in a server context, which is especially important for code that interacts with browser-specific APIs or relies on the presence of the window or document objects.- 63-65: The use of
extractDocumentAuthMethodsfunction to extract document authentication options is a clean approach to handling potentially complex logic outside of the component. This helps in keeping the component code more readable and maintainable. However, ensure thatextractDocumentAuthMethodsproperly handles all expected input cases, including null or undefineddocument.authOptions, to prevent runtime errors.- 67-78: The use of
useFormfromreact-hook-formwithzodResolverfor form validation is a robust choice for managing form state and validation. ThedefaultValuesare correctly set from thedocumentprop, ensuring that the form is initialized with the current document settings. This approach enhances the user experience by pre-populating the form with existing values, making it easier for users to see and modify the current settings.- 89-93: The
useEffecthook is used to set the timezone to the user's local timezone if it hasn't been touched and the document hasn't been sent. This is a thoughtful detail that improves user experience by defaulting to a sensible value while allowing for manual override. However, consider adding a comment explaining whydocumentHasBeenSentis checked, as it might not be immediately clear to other developers why the timezone should not be updated after the document has been sent.packages/ui/primitives/document-flow/add-signers.tsx (2)
- 3-3: The addition of
useMemoanduseStatefrom React aligns with the introduction of new stateful logic and memoization in the component. This is a good practice for optimizing performance and managing component state effectively.- 170-359: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [177-410]
The form structure and handling of advanced settings are well-organized and modular, making the code easy to read and maintain. The use of conditional rendering based on
showAdvancedSettingsto display advanced authentication options is a good practice. However, consider abstracting the repetitive logic for form fields into a separate component or function to reduce the complexity and improve modularity.
apps/web/src/app/(signing)/sign/[token]/signing-field-container.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/(signing)/sign/[token]/signing-field-container.tsx
Outdated
Show resolved
Hide resolved
|
Note Since we're bringing the advanced tab from the |
| import { seedUser, unseedUser } from '@documenso/prisma/seed/users'; | ||
|
|
||
| import { apiSignin } from '../fixtures/authentication'; | ||
|
|
||
| dotenv.config({ path: path.resolve(__dirname, '../../../../', '.env.local') }); |
There was a problem hiding this comment.
Should be fine to load the ENV into playwright right?
There was a problem hiding this comment.
Although it should already have the env for the most part 🤔
There was a problem hiding this comment.
Oh, seems like it was commented out 6 months ago
https://github.com/documenso/documenso/blob/main/packages/app-tests/playwright.config.ts#L7
I'll reinstate it
|
Thank you for following the naming conventions for pull request titles! 💚🚀 |
|
full trust |
Description
Add support for two types of authentication on a document:
Currently supported auth methods:
Passkeys will be added in at a later date.
Global document access and action authentication
These options allow us to configure the access and action requirements for all recipients in the document in one easy location.
Recipient action authentication
This setting allows us to individually set the action authentication requirements for a specific recipient. This overrides the global settings for more control.
Recipient access authentication
Same as above but for access. Currently not available via UI.
Changes Made
Notes
Since we're bringing the advanced tab from the
Add subjectdocument step to the first step, I've removed the restriction where we display the date dropdown only when a date field is configured, because it's early in the step.Testing Performed
Pending tests
Checklist
Summary by CodeRabbit