[WEB-4434] chore: disallowing special characters in user first name and last name #7953
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded a module-level forbidden-character regex and checks to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client (submit / front-end)
participant F as Front-end Validation
participant S as Backend Serializer
participant R as Regex (FORBIDDEN_NAME_CHARS_PATTERN)
C->>F: input first_name / last_name
F->>F: test against /^[a-zA-Z\s\-_]*$/
alt front-end pattern fails
F-->>C: show validation error (allowed characters)
else passes
F->>S: send payload
S->>R: re.search(FORBIDDEN_NAME_CHARS_PATTERN, value)
alt forbidden chars found
R-->>S: match
S-->>C: raise ValidationError("special characters not allowed")
else no forbidden chars
alt contains URL
S-->>C: raise ValidationError("URLs are not allowed")
else valid
S-->>C: accept / return value
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
sangeethailango
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/plane/app/serializers/user.py (1)
10-11: Improve import organization.The
remodule is part of Python's standard library, not a Django import. Standard library imports should be placed at the top of the file, before third-party imports, according to PEP 8.Apply this diff to reorganize the imports:
+# Python imports +import re + # Third party imports from rest_framework import serializers # Module import from plane.db.models import Account, Profile, User, Workspace, WorkspaceMemberInvite from plane.utils.url import contains_url from .base import BaseSerializer - -# Django imports -import re -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/serializers/user.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/app/serializers/user.py (1)
apps/api/plane/utils/url.py (1)
contains_url(22-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint API
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation to prevent special characters in user first names by implementing a regular expression check in the UserSerializer validation method.
- Adds regex validation to reject first names containing special characters like &, +, :, ;, =, ?, @, #, |, ', <, >, ., (, ), %, !, -
- Imports the
remodule to support regular expression operations - Maintains existing URL validation while adding the new character restriction
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api/plane/app/serializers/user.py (1)
17-17: Simplify regex pattern for efficiency.The pattern
^.*[...].*$uses redundant anchors and wildcards. Sincere.match()already matches from the start, the^is unnecessary. The pattern can be simplified.Apply this diff:
-FORBIDDEN_NAME_CHARS_PATTERN = r"^.*[&+,:;$=?@#|'<>.()%!-].*$" +FORBIDDEN_NAME_CHARS_PATTERN = r"[&+,:;$=?@#|'<>.()%!-]"Then update the validation methods to use
re.search()instead ofre.match():- if re.match(FORBIDDEN_NAME_CHARS_PATTERN, value): + if re.search(FORBIDDEN_NAME_CHARS_PATTERN, value):This achieves the same result with better performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/serializers/user.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/app/serializers/user.py (1)
apps/api/plane/utils/url.py (1)
contains_url(22-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/app/serializers/user.py (1)
1-2: LGTM!The
remodule import is now correctly placed with other Python standard library imports.
sangeethailango
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/plane/app/serializers/user.py (2)
2-2: Move import to match the existing comment structure.The
reimport is a standard Python library, not a Django import. It should be placed in the Python imports section (above line 4) instead of appearing after the "# Django imports" comment.Apply this diff to correct the import placement:
# Python imports import re - - + # Third party imports
17-17: Simplify regex pattern for better readability.The pattern
^.*[...].*$is unnecessarily complex. The anchors^.*and.*$are redundant when usingre.match(), which already matches from the start. The simpler pattern[...]achieves the same result and is more efficient.Apply this diff (after adding the hyphen):
-FORBIDDEN_NAME_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!\-].*$" +FORBIDDEN_NAME_CHARS_PATTERN = r"[&+,:;$^}{*=?@#|'<>.()%!\-]"Then update the validation to use
re.search()instead ofre.match():- if re.match(FORBIDDEN_NAME_CHARS_PATTERN, value): + if re.search(FORBIDDEN_NAME_CHARS_PATTERN, value):This change makes the pattern clearer and more efficient while maintaining the same functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/serializers/user.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/app/serializers/user.py (2)
apps/api/plane/api/serializers/base.py (1)
BaseSerializer(5-114)apps/api/plane/utils/url.py (1)
contains_url(22-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/api/plane/app/serializers/user.py (3)
25-26: Good addition! Validation logic is correct.The validation correctly checks for forbidden characters and provides a clear error message. The punctuation and capitalization are consistent with the URL validation error message.
34-35: Good addition! Consistent validation applied to last name.The last name validation now mirrors the first name validation, ensuring consistency across both fields. The error message formatting is appropriate.
14-17: Include hyphen in forbidden-name regex and review apostrophe ruleUpdate the comment and pattern to block
-:-# & + , : ; $ ^ } { * = ? @ # | ' < > . ( ) % ! +# & + , : ; $ ^ } { * = ? @ # | ' < > . ( ) % ! - -FORBIDDEN_NAME_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!].*$" +FORBIDDEN_NAME_CHARS_PATTERN = r"^.*[&+,:;$^}{*=?@#|'<>.()%!\-].*$"If you intend to allow apostrophes (e.g. O’Brien), remove
'from the character class and manually verify no existing users rely on it.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/core/components/profile/form.tsx (1)
312-322: Consider applying consistent character validation to display_name.The
display_namefield (lines 312-322) has length and whitespace validation but no character restrictions, whilefirst_nameandlast_namenow block special characters. If the goal is to prevent special characters in user-visible names,display_nameshould have similar validation.If consistency is desired, add pattern validation to
display_name:rules={{ required: "Display name is required.", + pattern: { + value: /^[\p{L}\p{M} \-_]*$/u, + message: "Display name can only contain letters, spaces, hyphens, and underscores", + }, validate: (value) => { if (value.trim().length < 1) return "Display name can't be empty.";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/onboarding/steps/profile/root.tsx(1 hunks)apps/web/core/components/profile/form.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/core/components/profile/form.tsx (1)
302-302: Good addition of error feedback for last_name validation.The error message display for
last_namebrings it into parity withfirst_name(line 273), ensuring consistent user experience when validation fails.apps/web/core/components/onboarding/steps/profile/root.tsx (1)
217-220: Confirm alignment of client-side regex with server validation
Couldn’t locateFORBIDDEN_NAME_CHARS_PATTERNon the server—verify that the server’s name validation matches the client regex^[a-zA-Z\s\-_]*$.
| pattern: { | ||
| value: /^[a-zA-Z\s\-_]*$/, | ||
| message: "Name can only contain letters, spaces, hyphens, and underscores", | ||
| }, |
There was a problem hiding this comment.
Regex pattern may be overly restrictive for international names and allows unwanted whitespace.
The pattern /^[a-zA-Z\s\-_]*$/ successfully blocks the listed special characters but has two concerns:
-
International names: The whitelist approach blocks accented characters (é, ñ, ü), non-Latin scripts (Cyrillic, Arabic, Chinese), and apostrophes commonly found in names like "O'Brien" or "D'Angelo". This could create friction for international users.
-
Whitespace matching: Using
\smatches all whitespace characters including tabs (\t) and newlines (\n), not just regular spaces. Names like"John\t\tDoe"would be accepted.
Consider these improvements:
For international support, use Unicode letter categories:
- pattern: {
- value: /^[a-zA-Z\s\-_]*$/,
- message: "Name can only contain letters, spaces, hyphens, and underscores",
- },
+ pattern: {
+ value: /^[\p{L}\p{M} \-_]*$/u,
+ message: "Name can only contain letters, spaces, hyphens, and underscores",
+ },For precise whitespace, replace \s with literal space:
- pattern: {
- value: /^[a-zA-Z\s\-_]*$/,
- message: "Name can only contain letters, spaces, hyphens, and underscores",
- },
+ pattern: {
+ value: /^[a-zA-Z \-_]*$/,
+ message: "Name can only contain letters, spaces, hyphens, and underscores",
+ },Or combine both for international support with controlled whitespace:
- pattern: {
- value: /^[a-zA-Z\s\-_]*$/,
- message: "Name can only contain letters, spaces, hyphens, and underscores",
- },
+ pattern: {
+ value: /^[\p{L}\p{M} \-_]*$/u,
+ message: "Name can only contain letters, spaces, hyphens, and underscores",
+ },Note: \p{L} matches Unicode letters, \p{M} matches combining marks (accents), and the u flag enables Unicode support.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pattern: { | |
| value: /^[a-zA-Z\s\-_]*$/, | |
| message: "Name can only contain letters, spaces, hyphens, and underscores", | |
| }, | |
| pattern: { | |
| value: /^[\p{L}\p{M} \-_]*$/u, | |
| message: "Name can only contain letters, spaces, hyphens, and underscores", | |
| }, |
🤖 Prompt for AI Agents
In apps/web/core/components/onboarding/steps/profile/root.tsx around lines 217
to 220, the current regex /^[a-zA-Z\s\-_]*$/ is too restrictive for
international names and allows tabs/newlines via \s; replace it with a
Unicode-aware pattern that permits Unicode letters and combining marks, explicit
spaces only, and common name punctuation (apostrophe, hyphen, underscore) and
ensure the regex uses the Unicode flag: use an anchored pattern like
^[\p{L}\p{M}\'\-\_ ]*$ with the u flag (or equivalent RegExp construction), and
update any validation messages accordingly so names with accented/non-Latin
letters and normal spaces are accepted while tabs/newlines remain rejected.
There was a problem hiding this comment.
looks like these changes are valid make these changes. @sangeethailango
There was a problem hiding this comment.
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!
| pattern: { | ||
| value: /^[a-zA-Z\s\-_]*$/, | ||
| message: "First name can only contain letters, spaces, hyphens, and underscores", | ||
| }, |
There was a problem hiding this comment.
Same pattern concerns: international names and whitespace handling.
Both first_name and last_name use the same regex /^[a-zA-Z\s\-_]*$/ which has the following concerns:
-
International names: Blocks accented characters (é, ñ, ü), non-Latin scripts, and apostrophes in names like "O'Brien". This affects international users.
-
Whitespace: Using
\sallows tabs and newlines, not just spaces.
Apply the same improvements suggested for the onboarding component:
rules={{
required: "Please enter first name",
- pattern: {
- value: /^[a-zA-Z\s\-_]*$/,
- message: "First name can only contain letters, spaces, hyphens, and underscores",
- },
+ pattern: {
+ value: /^[\p{L}\p{M} \-_]*$/u,
+ message: "First name can only contain letters, spaces, hyphens, and underscores",
+ },
}} rules={{
- pattern: {
- value: /^[a-zA-Z\s\-_]*$/,
- message: "Last name can only contain letters, spaces, hyphens, and underscores",
- },
+ pattern: {
+ value: /^[\p{L}\p{M} \-_]*$/u,
+ message: "Last name can only contain letters, spaces, hyphens, and underscores",
+ },
}}Also applies to: 280-285
Description
This PR will throw a validation error if the following special characters are given for the user's first name and last name
&+,:;$^}{*=?@#|'<>.()%!Type of Change
Summary by CodeRabbit