fix(api): accept avatar URLs and use email for user creation #25629
+288
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- avatarValidator: accept URLs in addition to base64 images
- organizations-users-service: use email for user creation instead of username
The avatarValidator was incorrectly rejecting valid URLs even though the API documentation shows URL examples and all e2e tests use URLs.
The user creation was failing when username was provided because createNewUsersConnectToOrgIfExists requires a valid email, not username. The username is correctly applied via updateOrganizationUser after creation.
Fixes user creation endpoint POST /v2/organizations/{orgId}/users
apps/api/v2/src/modules/organizations/users/index/services/organizations-users-service.ts
Show resolved
Hide resolved
keithwillcode
requested changes
Dec 5, 2025
Contributor
keithwillcode
left a comment
keithwillcode
left a comment
There was a problem hiding this comment.
Since logic is changing, we should have automated tests to verify the changes
supalarry
reviewed
Dec 5, 2025
apps/api/v2/src/modules/organizations/users/index/services/organizations-users-service.ts
Show resolved
Hide resolved
Add comprehensive test coverage for avatarValidator changes: - Unit tests: 18 scenarios covering valid/invalid URLs and base64 images - E2E tests: user creation with username + avatarUrl (URL and base64) - Security validation: reject unsafe protocols and malformed data - Error message verification for validation failures Addresses testing requirements from code review
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
pedroccastro
requested review from
hariombalhara,
keithwillcode,
sean-brydon and
supalarry
- Reject HTTP URLs, accept only HTTPS for security and browser compatibility - Reject empty strings and whitespace (use null to reset avatar) - Update validation message to clarify HTTPS requirement - Add test coverage for new security restrictions Addresses security feedback from code review regarding mixed content vulnerabilities and clearer API semantics for avatar reset
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/modules/users/validators/avatarValidator.ts">
<violation number="1" location="apps/api/v2/src/modules/users/validators/avatarValidator.ts:19">
P1: This change removes HTTP URL support, contradicting the PR description which states "Accept both URLs (http/https)". If the original endpoint accepted HTTP URLs, this is a breaking change for existing API consumers. Either update the code to accept HTTP or clarify in the PR description that HTTP is intentionally being removed for security reasons.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| private isValidUrl(value: string): boolean { | ||
| try { | ||
| const url = new URL(value); | ||
| return url.protocol === "https:"; |
Contributor
There was a problem hiding this comment.
P1: This change removes HTTP URL support, contradicting the PR description which states "Accept both URLs (http/https)". If the original endpoint accepted HTTP URLs, this is a breaking change for existing API consumers. Either update the code to accept HTTP or clarify in the PR description that HTTP is intentionally being removed for security reasons.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/modules/users/validators/avatarValidator.ts, line 19:
<comment>This change removes HTTP URL support, contradicting the PR description which states "Accept both URLs (http/https)". If the original endpoint accepted HTTP URLs, this is a breaking change for existing API consumers. Either update the code to accept HTTP or clarify in the PR description that HTTP is intentionally being removed for security reasons.</comment>
<file context>
@@ -7,15 +7,16 @@ const BASE64_IMAGE_REGEX =
try {
const url = new URL(value);
- return url.protocol === "http:" || url.protocol === "https:";
+ return url.protocol === "https:";
} catch {
return false;
</file context>
Suggested change
| return url.protocol === "https:"; | |
| return url.protocol === "http:" || url.protocol === "https:"; |
hariombalhara
approved these changes
Dec 6, 2025
| const regex = /^data:image\/[^;]+;base64,(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/; | ||
| return regex.test(avatarString); | ||
| validate(avatarString: string): boolean { | ||
| if (avatarString === null || avatarString === undefined) return true; |
Member
There was a problem hiding this comment.
nit: TS confirms at line 9 that avatarString can be string only.
So, we just need
if (!avatarString?.trim()) return false;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR fixes two bugs in API v2 user creation endpoint
POST /v2/organizations/{orgId}/users:Bug 1 - avatarValidator rejecting URLs:
avatarValidator: ""constraint errorBug 2 - User creation failing when username provided:
createNewUsersConnectToOrgIfExistsrequires a valid email"Invite failed because {username} is not a valid email address"Key Changes:
avatarValidator.ts: Accept both URLs (http/https) and base64 imagesorganizations-users-service.ts: Always use email for user creation; username is applied viaupdateOrganizationUserafter creationVisual Demo
Before - Avatar URL rejected
Before - Username causing TRPCError
After - Full payload working
How should this be tested?
Test Scenario 1 - Avatar URL accepted:
POST /v2/organizations/{orgId}/users{ "email": "test@example.com", "avatarUrl": "https://avatars.githubusercontent.com/u/583231?v=4" }Test Scenario 2 - Username with avatar:
POST /v2/organizations/{orgId}/users{ "email": "test@example.com", "username": "testuser", "avatarUrl": "https://avatars.githubusercontent.com/u/583231?v=4", "locale": "pt", "timeZone": "America/Sao_Paulo", "timeFormat": 24 }Test Scenario 3 - Minimal payload (regression):
POST /v2/organizations/{orgId}/users{ "email": "minimal@example.com" }Mandatory Tasks (DO NOT REMOVE)