fix(api): accept avatar URLs and use email for user creation (#25629)

* fix(api): fix user creation with avatarUrl and username

- 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

* test(api): add tests for avatar validator fixes

  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

* fix(api): enhance avatar validator security per code review

  - 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

* refactor(api): simplify avatarValidator null check

Remove redundant null/undefined check since @IsOptional decorator already skips validation for undefined values. Updated test mock to include @IsOptional to match real DTO usage

---------

Co-authored-by: Keith Williams <keithwillcode@gmail.com>
This commit is contained in:
Pedro CastroandKeith Williams authored and GitHub committed 2025-12-23 15:07:14 +00:00
1 parent 866c8c046d
commit 7847177e8b
4 files changed
+287 -10

No files matched your search

@@ -167,7 +167,7 @@ describe("Organizations Users Endpoints", () => {
await userRepositoryFixture.create({
email: nonMemberEmail,
username: "non-member",
username: `non-member-${randomString()}`,
});
const orgMembers = await Promise.all(
@@ -379,6 +379,108 @@ describe("Organizations Users Endpoints", () => {
createdUser = userData;
});
it("creates a new org user with username and avatarUrl", async () => {
const shortRandom = randomString().substring(0, 8);
const testUsername = `user${shortRandom}`;
const testEmail = `org-user-${shortRandom}@api.com`;
const githubAvatarUrl = "https://avatars.githubusercontent.com/u/583231?v=4";
const newOrgUserWithUsernameAndAvatar: CreateUserInput = {
email: testEmail,
username: testUsername,
avatarUrl: githubAvatarUrl,
bio,
metadata,
timeZone: "America/Sao_Paulo",
timeFormat: 24,
locale: "pt",
};
const emailSpy = jest
.spyOn(EmailService.prototype, "sendSignupToOrganizationEmail")
.mockImplementation(() => Promise.resolve());
const { body } = await request(app.getHttpServer())
.post(`/v2/organizations/${org.id}/users`)
.send(newOrgUserWithUsernameAndAvatar)
.set("Content-Type", "application/json")
.set("Accept", "application/json");
const userData = body.data;
// Verify response status
expect(body.status).toBe(SUCCESS_STATUS);
// Verify all user fields are correctly set
expect(userData.email).toBe(testEmail);
expect(userData.username).toBe(testUsername);
expect(userData.avatarUrl).toBe(githubAvatarUrl);
expect(userData.bio).toBe(bio);
expect(userData.metadata).toEqual(metadata);
expect(userData.timeZone).toBe("America/Sao_Paulo");
expect(userData.timeFormat).toBe(24);
expect(userData.locale).toBe("pt");
// Verify email was sent with correct parameters (using email, not username)
expect(emailSpy).toHaveBeenCalledWith({
usernameOrEmail: testEmail,
orgName: org.name,
orgId: org.id,
inviterName: userEmail,
locale: "pt",
});
// Clean up the created user
await userRepositoryFixture.deleteByEmail(testEmail);
});
it("creates a new org user with avatarUrl as base64 image", async () => {
const shortRandom = randomString().substring(0, 8);
const testEmail = `org-b64-${shortRandom}@api.com`;
const base64Avatar =
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==";
const newOrgUserWithBase64Avatar: CreateUserInput = {
email: testEmail,
avatarUrl: base64Avatar,
bio,
metadata,
};
const emailSpy = jest
.spyOn(EmailService.prototype, "sendSignupToOrganizationEmail")
.mockImplementation(() => Promise.resolve());
const { body } = await request(app.getHttpServer())
.post(`/v2/organizations/${org.id}/users`)
.send(newOrgUserWithBase64Avatar)
.set("Content-Type", "application/json")
.set("Accept", "application/json");
const userData = body.data;
// Verify response status
expect(body.status).toBe(SUCCESS_STATUS);
// Verify base64 avatar is accepted and stored
expect(userData.email).toBe(testEmail);
expect(userData.avatarUrl).toBe(base64Avatar);
expect(userData.bio).toBe(bio);
expect(userData.metadata).toEqual(metadata);
// Verify email was sent
expect(emailSpy).toHaveBeenCalledWith({
usernameOrEmail: testEmail,
orgName: org.name,
orgId: org.id,
inviterName: userEmail,
locale: null,
});
// Clean up the created user
await userRepositoryFixture.deleteByEmail(testEmail);
});
it("should delete an org user", async () => {
const { body } = await request(app.getHttpServer())
.delete(`/v2/organizations/${org.id}/users/${createdUser.id}`)
@@ -68,13 +68,11 @@ export class OrganizationsUsersService {
await this.checkForUsernameConflicts(org.id, userCreateBody.username);
}
const usernameOrEmail = userCreateBody.username ? userCreateBody.username : userCreateBody.email;
// Create new org user
const createdUserCall = await createNewUsersConnectToOrgIfExists({
invitations: [
{
usernameOrEmail: usernameOrEmail,
usernameOrEmail: userCreateBody.email,
role: userCreateBody.organizationRole,
},
],
@@ -84,7 +82,7 @@ export class OrganizationsUsersService {
parentId: null,
autoAcceptEmailDomain: "not-required-for-this-endpoint",
orgConnectInfoByUsernameOrEmail: {
[usernameOrEmail]: {
[userCreateBody.email]: {
orgId: org.id,
autoAccept: userCreateBody.autoAccept,
},
@@ -106,7 +104,7 @@ export class OrganizationsUsersService {
// Need to send email to new user to create password
await this.emailService.sendSignupToOrganizationEmail({
usernameOrEmail,
usernameOrEmail: userCreateBody.email,
orgName: org.name,
orgId: org.id,
locale: user?.locale,
@@ -0,0 +1,158 @@
import { IsOptional, Validate, validateSync } from "class-validator";
import { AvatarValidator } from "./avatarValidator";
// Mock DTO for testing the validator
class MockAvatarDto {
@IsOptional()
@Validate(AvatarValidator)
avatar?: string;
}
// Factory function to create DTOs with avatar
const createAvatarDto = (avatar?: string): MockAvatarDto => {
const dto = new MockAvatarDto();
dto.avatar = avatar;
return dto;
};
describe("AvatarValidator", () => {
describe("when value is undefined", () => {
it("accepts undefined values", () => {
const dto = createAvatarDto(undefined);
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
});
describe("when value is a URL", () => {
it("rejects HTTP URLs (HTTPS only for security)", () => {
const dto = createAvatarDto("http://example.com/avatar.jpg");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("accepts valid HTTPS URLs", () => {
const dto = createAvatarDto("https://avatars.githubusercontent.com/u/583231?v=4");
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("accepts URLs with query parameters", () => {
const dto = createAvatarDto("https://example.com/avatar.jpg?size=256&format=png");
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("accepts URLs with special characters in path", () => {
const dto = createAvatarDto("https://cdn.example.com/users/john-doe/avatar_2025.jpg");
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("rejects URLs without protocol", () => {
const dto = createAvatarDto("example.com/avatar.jpg");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects FTP protocol for security", () => {
const dto = createAvatarDto("ftp://example.com/avatar.jpg");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects file protocol for security", () => {
const dto = createAvatarDto("file:///path/to/avatar.jpg");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects javascript protocol to prevent XSS", () => {
const dto = createAvatarDto("javascript:alert('XSS')");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
});
describe("when value is base64 encoded image", () => {
it("accepts valid PNG base64", () => {
const dto = createAvatarDto(
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg=="
);
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("accepts valid JPEG base64", () => {
const dto = createAvatarDto("data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDAA==");
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("accepts valid GIF base64", () => {
const dto = createAvatarDto(
"data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=="
);
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("accepts valid WEBP base64", () => {
const dto = createAvatarDto(
"data:image/webp;base64,UklGRiQAAABXRUJQVlA4IBgAAAAwAQCdASoBAAEAAwA0JaQAA3AA/vuUAAA="
);
const errors = validateSync(dto);
expect(errors).toHaveLength(0);
});
it("rejects base64 with non-image MIME type", () => {
const dto = createAvatarDto("data:text/plain;base64,SGVsbG8gV29ybGQ=");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects malformed base64 data URL", () => {
const dto = createAvatarDto("data:image/png;base64,INVALID_BASE64_@#$%");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects base64 with application/json MIME type", () => {
const dto = createAvatarDto("data:application/json;base64,eyJmb28iOiJiYXIifQ==");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
});
describe("when value is invalid", () => {
it("rejects plain text strings", () => {
const dto = createAvatarDto("not a url or base64");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects empty strings (use null to reset avatar)", () => {
const dto = createAvatarDto("");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
it("rejects whitespace-only strings", () => {
const dto = createAvatarDto(" ");
const errors = validateSync(dto);
expect(errors).toHaveLength(1);
expect(errors[0].constraints).toHaveProperty("avatarValidator");
});
});
});
@@ -1,11 +1,30 @@
import { ValidatorConstraint } from "class-validator";
import type { ValidatorConstraintInterface } from "class-validator";
const BASE64_IMAGE_REGEX =
/^data:image\/[^;]+;base64,(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/;
@ValidatorConstraint({ name: "avatarValidator", async: false })
export class AvatarValidator implements ValidatorConstraintInterface {
validate(avatarString: string) {
// Checks if avatar string is a valid base 64 image
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?.trim()) return false;
return this.isValidUrl(avatarString) || this.isValidBase64Image(avatarString);
}
private isValidUrl(value: string): boolean {
try {
const url = new URL(value);
return url.protocol === "https:";
} catch {
return false;
}
}
private isValidBase64Image(value: string): boolean {
return BASE64_IMAGE_REGEX.test(value);
}
defaultMessage(): string {
return "avatarUrl must be a valid HTTPS URL (HTTP not allowed) or base64 encoded image";
}
}