chore: added approved messages #40108
Conversation
WalkthroughThe update introduces a dedicated module for approved error messages. A new file defines a set of static messages and regex patterns alongside functions to validate and sanitize error messages. Existing message constants have been standardized with consistent apostrophe usage and enriched with new authentication-related messages. Additionally, user authentication components now utilize the safe error message functions, and a re-export file supports enterprise usage. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant Safe as getSafeErrorMessage
participant Checker as isApprovedErrorMessage
participant Creator as createMessage
UI->>Safe: getSafeErrorMessage(errorMessage)
Safe->>Checker: isApprovedErrorMessage(errorMessage)
Checker-->>Safe: Approval result (true/false)
alt Message Approved
Safe-->>UI: Return approved (wrapped) error message
else Message Not Approved
Safe->>Creator: createMessage(default)
Creator-->>Safe: Default error message
Safe-->>UI: Return safe error message
end
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (1)
🪧 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/UserAuth/Login.tsx (1)
56-56: Import path inconsistent with other files.Other files are importing from "ee/constants/approvedErrorMessages", but this file imports directly from "ce/constants/approvedErrorMessages". For consistency, consider using the EE path here as well.
-import { getSafeErrorMessage } from "ce/constants/approvedErrorMessages"; +import { getSafeErrorMessage } from "ee/constants/approvedErrorMessages";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/ce/constants/approvedErrorMessages.ts(1 hunks)app/client/src/ce/constants/messages.ts(30 hunks)app/client/src/ee/constants/approvedErrorMessages.ts(1 hunks)app/client/src/pages/UserAuth/Login.tsx(2 hunks)app/client/src/pages/UserAuth/SignUp.tsx(2 hunks)app/client/src/pages/UserAuth/VerificationError.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
app/client/src/pages/UserAuth/Login.tsx (1)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
getSafeErrorMessage(111-129)
app/client/src/pages/UserAuth/VerificationError.tsx (1)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
getSafeErrorMessage(111-129)
app/client/src/pages/UserAuth/SignUp.tsx (1)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
getSafeErrorMessage(111-129)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
app/client/src/ce/constants/messages.ts (30)
ERROR_401(154-155)AUTH_LOGIN_TOO_MANY_ATTEMPTS(2658-2658)AUTH_INVALID_CREDENTIALS(2659-2659)AUTH_UNAUTHORIZED(2660-2660)AUTH_NOT_LOGGED_IN(2661-2661)AUTH_RESET_PASSWORD_REQUEST_INVALID(2662-2662)AUTH_LOGIN_METHOD_NOT_SUPPORTED(2663-2663)AUTH_ACCOUNT_LOCKED(2664-2664)AUTH_SESSION_INVALID(2665-2665)LOGIN_PAGE_INVALID_CREDS_ERROR(62-63)FORM_VALIDATION_PASSWORD_RULE(53-54)FORM_VALIDATION_INVALID_PASSWORD(55-55)FORM_VALIDATION_EMPTY_PASSWORD(52-52)FORM_VALIDATION_EMPTY_EMAIL(46-46)FORM_VALIDATION_INVALID_EMAIL(47-48)RESET_PASSWORD_EXPIRED_TOKEN(110-111)RESET_PASSWORD_INVALID_TOKEN(112-113)FORGOT_PASSWORD_PAGE_SUBTITLE(122-123)SIGNUP_PAGE_SUCCESS(93-94)RESET_PASSWORD_RESET_SUCCESS(106-107)CREATE_PASSWORD_RESET_SUCCESS(235-235)WELCOME_FORM_PASSWORDS_NOT_MATCHING_ERROR_MESSAGE(1603-1604)WELCOME_FORM_STRONG_PASSWORD_ERROR_MESSAGE(1597-1598)VERIFY_ERROR_ALREADY_VERIFIED_TITLE(139-140)VERIFY_ERROR_EXPIRED_TITLE(142-142)EMAIL_VERIFICATION_NOT_ENABLED(2668-2668)VERIFY_ERROR_MISMATCH_TITLE(144-145)GOOGLE_RECAPTCHA_FAILED(2669-2669)PASSWORD_INSUFFICIENT_STRENGTH(2670-2670)DEFAULT_ERROR_MESSAGE(323-323)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (10)
app/client/src/ee/constants/approvedErrorMessages.ts (1)
1-1: Looks good - this is a standard re-export file.The file correctly re-exports all entities from the CE (Community Edition) module, making them available for Enterprise Edition code paths.
app/client/src/pages/UserAuth/Login.tsx (1)
179-180: Error message sanitization looks good.The implementation properly uses the new
getSafeErrorMessagefunction to sanitize error messages before display.app/client/src/pages/UserAuth/SignUp.tsx (2)
69-69: Import from EE module is correct.Good job using the Enterprise Edition module path for importing the function, maintaining proper architecture separation.
268-268: Error message sanitization implemented correctly.The implementation properly uses
getSafeErrorMessageto sanitize error messages before display.app/client/src/pages/UserAuth/VerificationError.tsx (2)
17-17: Import from EE module is correct.Good job using the Enterprise Edition module path for importing the function.
81-81: Error message sanitization with empty fallback.Good implementation that uses
getSafeErrorMessagewith a fallback to empty string if message is null or undefined.app/client/src/ce/constants/approvedErrorMessages.ts (3)
34-87: Good implementation of message whitelist pattern.The approach of categorizing approved messages into static and pattern-based collections is robust. It provides both security and flexibility for displaying error messages.
94-104: Efficient implementation for message validation.The function efficiently checks both static messages and regex patterns, with proper handling of message formats (string literals vs. function references).
111-129: Proper error message sanitization logic.The function ensures users only see approved error messages or a default fallback, which is a good security practice to prevent information leakage through error messages.
app/client/src/ce/constants/messages.ts (1)
2658-2670: Good addition of standardized authentication error messages.These new authentication-related constants provide consistent error messaging throughout the application. The messages are user-friendly and informative without revealing sensitive implementation details.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/ce/constants/approvedErrorMessages.ts (3)
35-89: Well-structured categorization of approved error messages.The separation between static messages and regex patterns is a good design choice. The organization within the static section (Authentication & Authorization, Signup Specific) makes the code more maintainable.
Consider adding a brief comment for each regex pattern explaining what kind of dynamic content it's intended to match, which would improve maintainability for future developers.
117-137: The error message sanitization logic is secure.The
getSafeErrorMessagefunction follows the security principle of allowing only explicitly approved messages to pass through, with a safe fallback to a generic message. The handling of different message types (function vs direct string) is handled appropriately.A minor optimization would be to extract the static message lookup logic into a separate helper function since it's used in both
isApprovedErrorMessageandgetSafeErrorMessage.+// Helper function to check if a message matches a static message +function matchesStaticMessage(message: string, staticMsg: Function | string): boolean { + return typeof staticMsg === "function" ? staticMsg() === message : staticMsg === message; +} export function isApprovedErrorMessage(message: string): boolean { // First check static messages if ( APPROVED_ERROR_MESSAGES.static.some((msg) => - typeof msg === "function" ? msg() === message : msg === message, + matchesStaticMessage(message, msg), ) ) { return true; } // Then check regex patterns return APPROVED_ERROR_MESSAGES.patterns.some((pattern) => pattern.test(message), ); } export function getSafeErrorMessage(message: string): string { const isApproved = isApprovedErrorMessage(message); if (!isApproved) { return createMessage(DEFAULT_ERROR_MESSAGE); } // If it's a static message, find and return it wrapped in createMessage const staticMessage = APPROVED_ERROR_MESSAGES.static.find((msg) => - typeof msg === "function" ? msg() === message : msg === message, + matchesStaticMessage(message, msg), );
1-137: Overall implementation enhances security and message consistency.This implementation effectively addresses potential security concerns by controlling which error messages can be displayed to users. It prevents information leakage through error messages while maintaining a good user experience with meaningful error messages.
Two suggestions for further improvement:
- Consider adding unit tests specifically for this module to ensure all error paths are covered.
- Monitor performance if the list of approved messages grows substantially large, as linear searches through large arrays can become inefficient.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
app/client/src/ce/constants/approvedErrorMessages.ts(1 hunks)app/client/src/ce/constants/messages.ts(30 hunks)app/client/src/ee/constants/approvedErrorMessages.ts(1 hunks)app/client/src/pages/UserAuth/SignUp.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/src/ee/constants/approvedErrorMessages.ts
- app/client/src/pages/UserAuth/SignUp.tsx
- app/client/src/ce/constants/messages.ts
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
app/client/src/ce/constants/messages.ts (30)
ERROR_401(154-155)AUTH_LOGIN_TOO_MANY_ATTEMPTS(2658-2659)AUTH_INVALID_CREDENTIALS(2660-2661)AUTH_UNAUTHORIZED(2662-2662)AUTH_NOT_LOGGED_IN(2663-2664)AUTH_RESET_PASSWORD_REQUEST_INVALID(2665-2666)AUTH_LOGIN_METHOD_NOT_SUPPORTED(2667-2668)AUTH_ACCOUNT_LOCKED(2669-2670)AUTH_SESSION_INVALID(2671-2672)LOGIN_PAGE_INVALID_CREDS_ERROR(62-63)FORM_VALIDATION_PASSWORD_RULE(53-54)FORM_VALIDATION_INVALID_PASSWORD(55-55)FORM_VALIDATION_EMPTY_PASSWORD(52-52)FORM_VALIDATION_EMPTY_EMAIL(46-46)FORM_VALIDATION_INVALID_EMAIL(47-48)RESET_PASSWORD_EXPIRED_TOKEN(110-111)RESET_PASSWORD_INVALID_TOKEN(112-113)FORGOT_PASSWORD_PAGE_SUBTITLE(122-123)SIGNUP_PAGE_SUCCESS(93-94)RESET_PASSWORD_RESET_SUCCESS(106-107)CREATE_PASSWORD_RESET_SUCCESS(235-235)WELCOME_FORM_PASSWORDS_NOT_MATCHING_ERROR_MESSAGE(1603-1604)WELCOME_FORM_STRONG_PASSWORD_ERROR_MESSAGE(1597-1598)VERIFY_ERROR_ALREADY_VERIFIED_TITLE(139-140)VERIFY_ERROR_EXPIRED_TITLE(142-142)EMAIL_VERIFICATION_NOT_ENABLED(2675-2676)VERIFY_ERROR_MISMATCH_TITLE(144-145)GOOGLE_RECAPTCHA_FAILED(2677-2678)PASSWORD_INSUFFICIENT_STRENGTH(2679-2680)DEFAULT_ERROR_MESSAGE(323-323)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (2)
app/client/src/ce/constants/approvedErrorMessages.ts (2)
1-33: Imports are well-organized and comprehensive.The imports are logically grouped and include all necessary message constants from the messages file needed for the approved error messages system.
96-110: The message validation logic is robust.The function correctly handles both function-style message constants and direct string constants. The two-step validation process (static messages first, then regex patterns) is efficient and clear.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/client/src/ce/constants/approvedErrorMessages.ts (2)
35-89: Well-structured approach for approved error messages.The separation between static messages and regex patterns provides a clear framework for message validation. The categorization with comments enhances readability.
Consider documenting the security implications of the regex patterns - particularly those capturing dynamic content like email addresses - to ensure future modifications don't introduce information leakage.
96-110: Consider performance optimization for the approval check.The function performs well but could be optimized by caching results for frequently checked messages, especially if called repeatedly in UI components.
+ // Cache for previously validated messages + const messageValidationCache = new Map<string, boolean>(); + export function isApprovedErrorMessage(message: string): boolean { + // Check cache first + if (messageValidationCache.has(message)) { + return messageValidationCache.get(message)!; + } + // First check static messages if ( APPROVED_ERROR_MESSAGES.static.some((msg) => typeof msg === "function" ? msg() === message : msg === message, ) ) { + messageValidationCache.set(message, true); return true; } // Then check regex patterns - return APPROVED_ERROR_MESSAGES.patterns.some((pattern) => + const result = APPROVED_ERROR_MESSAGES.patterns.some((pattern) => pattern.test(message), ); + messageValidationCache.set(message, result); + return result; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/ce/constants/approvedErrorMessages.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/ce/constants/approvedErrorMessages.ts (1)
app/client/src/ce/constants/messages.ts (30)
ERROR_401(154-155)AUTH_LOGIN_TOO_MANY_ATTEMPTS(2658-2659)AUTH_INVALID_CREDENTIALS(2660-2661)AUTH_UNAUTHORIZED(2662-2662)AUTH_NOT_LOGGED_IN(2663-2664)AUTH_RESET_PASSWORD_REQUEST_INVALID(2665-2666)AUTH_LOGIN_METHOD_NOT_SUPPORTED(2667-2668)AUTH_ACCOUNT_LOCKED(2669-2670)AUTH_SESSION_INVALID(2671-2672)LOGIN_PAGE_INVALID_CREDS_ERROR(62-63)FORM_VALIDATION_PASSWORD_RULE(53-54)FORM_VALIDATION_INVALID_PASSWORD(55-55)FORM_VALIDATION_EMPTY_PASSWORD(52-52)FORM_VALIDATION_EMPTY_EMAIL(46-46)FORM_VALIDATION_INVALID_EMAIL(47-48)RESET_PASSWORD_EXPIRED_TOKEN(110-111)RESET_PASSWORD_INVALID_TOKEN(112-113)FORGOT_PASSWORD_PAGE_SUBTITLE(122-123)SIGNUP_PAGE_SUCCESS(93-94)RESET_PASSWORD_RESET_SUCCESS(106-107)CREATE_PASSWORD_RESET_SUCCESS(235-235)WELCOME_FORM_PASSWORDS_NOT_MATCHING_ERROR_MESSAGE(1603-1604)WELCOME_FORM_STRONG_PASSWORD_ERROR_MESSAGE(1597-1598)VERIFY_ERROR_ALREADY_VERIFIED_TITLE(139-140)VERIFY_ERROR_EXPIRED_TITLE(142-142)EMAIL_VERIFICATION_NOT_ENABLED(2675-2676)VERIFY_ERROR_MISMATCH_TITLE(144-145)GOOGLE_RECAPTCHA_FAILED(2677-2678)PASSWORD_INSUFFICIENT_STRENGTH(2679-2680)DEFAULT_ERROR_MESSAGE(323-323)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (2)
app/client/src/ce/constants/approvedErrorMessages.ts (2)
1-33: Import organization looks good.All imports are used in the constants and functions below, maintaining clean dependencies.
117-137: Good defensive approach to error message handling.The function properly sanitizes messages and falls back to a default error if needed. The handling of different message types (function vs string) is well implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/ce/constants/messages.ts (2)
3-7: Consider addressing the TODO comment during this changeThere's a TODO comment at the top of the file that's been left unaddressed. Since you're already making changes to this file, it would be a good opportunity to handle this noted issue.
1-2683: Consider refactoring this large constants fileThis file is over 2500 lines long and continues to grow with each update. Consider splitting it into logical modules (e.g., auth-related messages, datasource messages, etc.) to improve maintainability.
A possible approach:
// In src/ce/constants/messages/auth.ts export const AUTH_LOGIN_TOO_MANY_ATTEMPTS = () => "Too many login attempts. Please try again after some time."; export const AUTH_INVALID_CREDENTIALS = () => "Invalid credentials provided. Did you input the credentials correctly?"; // ... other auth messages // In src/ce/constants/messages/index.ts export * from './auth'; export * from './datasource'; export * from './ui'; // ... etc.🧰 Tools
🪛 Biome (1.9.4)
[error] 1169-1169: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
app/client/src/ce/constants/messages.ts(30 hunks)app/client/src/pages/UserAuth/SignUp.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/UserAuth/SignUp.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/src/ce/constants/messages.ts (2)
163-163: Good standardization of apostrophesThe consistent replacement of curly apostrophes with straight apostrophes improves code consistency. This standardization makes the codebase more uniform and helps prevent potential string comparison issues.
Also applies to: 209-210, 390-390, 402-402, 597-597, 977-978, 1000-1000, 1254-1254, 1267-1268, 1334-1334, 1347-1347, 1587-1587, 1627-1627, 1726-1726, 1729-1730, 1736-1736, 1748-1748, 1759-1760, 1770-1770, 1816-1816, 1845-1845, 1870-1871, 1964-1964, 2064-2064, 2517-2517, 2590-2590, 2610-2610, 2619-2620, 2638-2638
2660-2683: Well-structured authentication error messagesThe new authentication error messages are clear, specific, and provide good guidance to users without revealing sensitive information. They follow security best practices by being informative without leaking internal details.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/ce/constants/messages.ts (1)
1-2690: Consider splitting this large constants file for better maintainability.This file is quite large (2690 lines) and contains many different types of messages. Consider splitting it into multiple files by category (authentication, UI, errors, etc.) to improve maintainability.
For example, you could create files like:
authMessages.tsfor authentication-related messagesuiMessages.tsfor UI-related messageserrorMessages.tsfor error messages- // One large file with all constants - export const AUTH_LOGIN_TOO_MANY_ATTEMPTS = () => "..."; - export const UI_ELEMENT_PANEL_SEARCH_TEXT = () => "Search UI elements"; - export const ERROR_MESSAGE_NAME_EMPTY = () => "Please select a name"; + // auth/authMessages.ts + export const AUTH_LOGIN_TOO_MANY_ATTEMPTS = () => "..."; + + // ui/uiMessages.ts + export const UI_ELEMENT_PANEL_SEARCH_TEXT = () => "Search UI elements"; + + // errors/errorMessages.ts + export const ERROR_MESSAGE_NAME_EMPTY = () => "Please select a name";Then create a barrel file (index.ts) to re-export everything if you need to maintain the current import structure.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1169-1169: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/ce/constants/messages.ts(30 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/src/ce/constants/messages.ts (2)
163-163: Apostrophe usage standardized to straight quotes.The apostrophe in this string has been standardized to use straight single quotes instead of curly quotes for consistency.
2667-2689: New authentication error message constants added.These new constants provide standardized error messages for various authentication scenarios, which improves consistency across the application and makes error handling more maintainable.
Description
Conversation link - https://theappsmith.slack.com/archives/C0341RERY4R/p1743064966404339
EE PR - https://github.com/appsmithorg/appsmith-ee/pull/6989
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14355493550
Commit: a3fdc2e
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 09 Apr 2025 12:22:30 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style