fix(SEC-WEB-27+28): Robust email validation + role cast validation
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
SEC-WEB-27: Replace weak email.includes('@') check with RFC 5322-aligned
programmatic validation (isValidEmail). Uses character-level domain label
validation to avoid ReDoS vulnerabilities from complex regex patterns.
SEC-WEB-28: Replace unsafe 'as WorkspaceMemberRole' type casts with
runtime validation (toWorkspaceMemberRole) that checks against known enum
values and falls back to MEMBER for invalid inputs. Applied in both
InviteMember.tsx and MemberList.tsx.
Adds 43 tests covering validation logic, InviteMember component, and
MemberList component behavior.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
109
apps/web/src/components/workspace/InviteMember.test.tsx
Normal file
109
apps/web/src/components/workspace/InviteMember.test.tsx
Normal file
@@ -0,0 +1,109 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
import { InviteMember } from "./InviteMember";
|
||||
|
||||
/**
|
||||
* Helper to get the invite form element from the rendered component.
|
||||
* The form wraps the submit button, so we locate it via the button.
|
||||
*/
|
||||
function getForm(): HTMLFormElement {
|
||||
const button = screen.getByRole("button", { name: /send invitation/i });
|
||||
const form = button.closest("form");
|
||||
if (!form) {
|
||||
throw new Error("Could not locate <form> element in InviteMember");
|
||||
}
|
||||
return form;
|
||||
}
|
||||
|
||||
describe("InviteMember", (): void => {
|
||||
const mockOnInvite = vi.fn<(email: string, role: WorkspaceMemberRole) => Promise<void>>();
|
||||
|
||||
beforeEach((): void => {
|
||||
mockOnInvite.mockReset();
|
||||
mockOnInvite.mockResolvedValue(undefined);
|
||||
vi.spyOn(window, "alert").mockImplementation((): undefined => undefined);
|
||||
});
|
||||
|
||||
it("should render the invite form", (): void => {
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
expect(screen.getByLabelText(/email address/i)).toBeInTheDocument();
|
||||
expect(screen.getByLabelText(/role/i)).toBeInTheDocument();
|
||||
expect(screen.getByRole("button", { name: /send invitation/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show error for empty email", async (): Promise<void> => {
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
fireEvent.submit(getForm());
|
||||
|
||||
expect(await screen.findByText("Email is required")).toBeInTheDocument();
|
||||
expect(mockOnInvite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should show error for invalid email without domain", async (): Promise<void> => {
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
const emailInput = screen.getByLabelText(/email address/i);
|
||||
fireEvent.change(emailInput, { target: { value: "notanemail" } });
|
||||
fireEvent.submit(getForm());
|
||||
|
||||
expect(await screen.findByText("Please enter a valid email address")).toBeInTheDocument();
|
||||
expect(mockOnInvite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should show error for email with only @ sign", async (): Promise<void> => {
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
const emailInput = screen.getByLabelText(/email address/i);
|
||||
fireEvent.change(emailInput, { target: { value: "user@" } });
|
||||
fireEvent.submit(getForm());
|
||||
|
||||
expect(await screen.findByText("Please enter a valid email address")).toBeInTheDocument();
|
||||
expect(mockOnInvite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should accept valid email and invoke onInvite", async (): Promise<void> => {
|
||||
const user = userEvent.setup();
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
await user.type(screen.getByLabelText(/email address/i), "valid@example.com");
|
||||
await user.click(screen.getByRole("button", { name: /send invitation/i }));
|
||||
|
||||
expect(mockOnInvite).toHaveBeenCalledWith("valid@example.com", WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should allow selecting a different role", async (): Promise<void> => {
|
||||
const user = userEvent.setup();
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
await user.type(screen.getByLabelText(/email address/i), "admin@example.com");
|
||||
await user.selectOptions(screen.getByLabelText(/role/i), WorkspaceMemberRole.ADMIN);
|
||||
await user.click(screen.getByRole("button", { name: /send invitation/i }));
|
||||
|
||||
expect(mockOnInvite).toHaveBeenCalledWith("admin@example.com", WorkspaceMemberRole.ADMIN);
|
||||
});
|
||||
|
||||
it("should show error message when onInvite rejects", async (): Promise<void> => {
|
||||
mockOnInvite.mockRejectedValueOnce(new Error("Invite failed"));
|
||||
const user = userEvent.setup();
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
await user.type(screen.getByLabelText(/email address/i), "user@example.com");
|
||||
await user.click(screen.getByRole("button", { name: /send invitation/i }));
|
||||
|
||||
expect(await screen.findByText("Invite failed")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should reset form after successful invite", async (): Promise<void> => {
|
||||
const user = userEvent.setup();
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
const emailInput = screen.getByLabelText(/email address/i);
|
||||
await user.type(emailInput, "user@example.com");
|
||||
await user.click(screen.getByRole("button", { name: /send invitation/i }));
|
||||
|
||||
expect(emailInput).toHaveValue("");
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useState } from "react";
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
import { isValidEmail, toWorkspaceMemberRole } from "./validation";
|
||||
|
||||
interface InviteMemberProps {
|
||||
onInvite: (email: string, role: WorkspaceMemberRole) => Promise<void>;
|
||||
@@ -22,7 +23,7 @@ export function InviteMember({ onInvite }: InviteMemberProps): React.JSX.Element
|
||||
return;
|
||||
}
|
||||
|
||||
if (!email.includes("@")) {
|
||||
if (!isValidEmail(email.trim())) {
|
||||
setError("Please enter a valid email address");
|
||||
return;
|
||||
}
|
||||
@@ -72,7 +73,7 @@ export function InviteMember({ onInvite }: InviteMemberProps): React.JSX.Element
|
||||
id="role"
|
||||
value={role}
|
||||
onChange={(e) => {
|
||||
setRole(e.target.value as WorkspaceMemberRole);
|
||||
setRole(toWorkspaceMemberRole(e.target.value));
|
||||
}}
|
||||
disabled={isInviting}
|
||||
className="w-full px-3 py-2 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent disabled:bg-gray-100"
|
||||
|
||||
109
apps/web/src/components/workspace/MemberList.test.tsx
Normal file
109
apps/web/src/components/workspace/MemberList.test.tsx
Normal file
@@ -0,0 +1,109 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
import { MemberList } from "./MemberList";
|
||||
import type { WorkspaceMemberWithUser } from "./MemberList";
|
||||
|
||||
const makeMember = (
|
||||
overrides: Partial<WorkspaceMemberWithUser> & { userId: string }
|
||||
): WorkspaceMemberWithUser => ({
|
||||
workspaceId: overrides.workspaceId ?? "ws-1",
|
||||
userId: overrides.userId,
|
||||
role: overrides.role ?? WorkspaceMemberRole.MEMBER,
|
||||
joinedAt: overrides.joinedAt ?? new Date("2025-01-01"),
|
||||
user: overrides.user ?? {
|
||||
id: overrides.userId,
|
||||
name: `User ${overrides.userId}`,
|
||||
email: `${overrides.userId}@example.com`,
|
||||
emailVerified: true,
|
||||
image: null,
|
||||
authProviderId: `auth-${overrides.userId}`,
|
||||
preferences: {},
|
||||
createdAt: new Date("2025-01-01"),
|
||||
updatedAt: new Date("2025-01-01"),
|
||||
},
|
||||
});
|
||||
|
||||
describe("MemberList", (): void => {
|
||||
const mockOnRoleChange = vi.fn<(userId: string, newRole: WorkspaceMemberRole) => Promise<void>>();
|
||||
const mockOnRemove = vi.fn<(userId: string) => Promise<void>>();
|
||||
|
||||
const defaultProps = {
|
||||
currentUserId: "user-1",
|
||||
currentUserRole: WorkspaceMemberRole.ADMIN,
|
||||
workspaceOwnerId: "owner-1",
|
||||
onRoleChange: mockOnRoleChange,
|
||||
onRemove: mockOnRemove,
|
||||
};
|
||||
|
||||
beforeEach((): void => {
|
||||
mockOnRoleChange.mockReset();
|
||||
mockOnRoleChange.mockResolvedValue(undefined);
|
||||
mockOnRemove.mockReset();
|
||||
mockOnRemove.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("should render member list with correct count", (): void => {
|
||||
const members = [makeMember({ userId: "user-1" }), makeMember({ userId: "user-2" })];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
expect(screen.getByText("Members (2)")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should display member name and email", (): void => {
|
||||
const members = [
|
||||
makeMember({
|
||||
userId: "user-2",
|
||||
user: {
|
||||
id: "user-2",
|
||||
name: "Jane Doe",
|
||||
email: "jane@example.com",
|
||||
emailVerified: true,
|
||||
image: null,
|
||||
authProviderId: "auth-2",
|
||||
preferences: {},
|
||||
createdAt: new Date("2025-01-01"),
|
||||
updatedAt: new Date("2025-01-01"),
|
||||
},
|
||||
}),
|
||||
];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
expect(screen.getByText("Jane Doe")).toBeInTheDocument();
|
||||
expect(screen.getByText("jane@example.com")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show (you) indicator for current user", (): void => {
|
||||
const members = [makeMember({ userId: "user-1" })];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
expect(screen.getByText("(you)")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should call onRoleChange with validated role when admin changes a member role", async (): Promise<void> => {
|
||||
const user = userEvent.setup();
|
||||
const members = [
|
||||
makeMember({ userId: "user-1" }),
|
||||
makeMember({ userId: "user-2", role: WorkspaceMemberRole.MEMBER }),
|
||||
];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
|
||||
const roleSelect = screen.getByDisplayValue("Member");
|
||||
await user.selectOptions(roleSelect, WorkspaceMemberRole.GUEST);
|
||||
|
||||
expect(mockOnRoleChange).toHaveBeenCalledWith("user-2", WorkspaceMemberRole.GUEST);
|
||||
});
|
||||
|
||||
it("should not show role select for the workspace owner", (): void => {
|
||||
const members = [
|
||||
makeMember({ userId: "owner-1", role: WorkspaceMemberRole.OWNER }),
|
||||
makeMember({ userId: "user-1", role: WorkspaceMemberRole.ADMIN }),
|
||||
];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
expect(screen.getByText("OWNER")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should not show remove button for the workspace owner", (): void => {
|
||||
const members = [makeMember({ userId: "owner-1", role: WorkspaceMemberRole.OWNER })];
|
||||
render(<MemberList {...defaultProps} members={members} />);
|
||||
expect(screen.queryByLabelText("Remove member")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import type { User, WorkspaceMember } from "@mosaic/shared";
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
import { toWorkspaceMemberRole } from "./validation";
|
||||
|
||||
export interface WorkspaceMemberWithUser extends WorkspaceMember {
|
||||
user: User;
|
||||
@@ -88,7 +89,7 @@ export function MemberList({
|
||||
<select
|
||||
value={member.role}
|
||||
onChange={(e) =>
|
||||
handleRoleChange(member.userId, e.target.value as WorkspaceMemberRole)
|
||||
handleRoleChange(member.userId, toWorkspaceMemberRole(e.target.value))
|
||||
}
|
||||
className="px-3 py-1 border border-gray-300 rounded-lg text-sm focus:ring-2 focus:ring-blue-500 focus:border-transparent"
|
||||
>
|
||||
|
||||
134
apps/web/src/components/workspace/validation.test.ts
Normal file
134
apps/web/src/components/workspace/validation.test.ts
Normal file
@@ -0,0 +1,134 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
import { isValidEmail, toWorkspaceMemberRole } from "./validation";
|
||||
|
||||
describe("isValidEmail", (): void => {
|
||||
describe("valid emails", (): void => {
|
||||
it("should accept a standard email", (): void => {
|
||||
expect(isValidEmail("user@example.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with dots in local part", (): void => {
|
||||
expect(isValidEmail("first.last@example.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with plus addressing", (): void => {
|
||||
expect(isValidEmail("user+tag@example.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with subdomain", (): void => {
|
||||
expect(isValidEmail("user@mail.example.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with hyphen in domain", (): void => {
|
||||
expect(isValidEmail("user@my-domain.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with special characters in local part", (): void => {
|
||||
expect(isValidEmail("user!#$%&'*+/=?^_`{|}~@example.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept email with numbers", (): void => {
|
||||
expect(isValidEmail("user123@example456.com")).toBe(true);
|
||||
});
|
||||
|
||||
it("should accept single-character local part", (): void => {
|
||||
expect(isValidEmail("a@example.com")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid emails", (): void => {
|
||||
it("should reject empty string", (): void => {
|
||||
expect(isValidEmail("")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email without @", (): void => {
|
||||
expect(isValidEmail("userexample.com")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email with only @", (): void => {
|
||||
expect(isValidEmail("@")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email without local part", (): void => {
|
||||
expect(isValidEmail("@example.com")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email without domain", (): void => {
|
||||
expect(isValidEmail("user@")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email with spaces", (): void => {
|
||||
expect(isValidEmail("user @example.com")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email with multiple @ signs", (): void => {
|
||||
expect(isValidEmail("user@@example.com")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email with domain starting with hyphen", (): void => {
|
||||
expect(isValidEmail("user@-example.com")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email exceeding 254 characters", (): void => {
|
||||
const longLocal = "a".repeat(243);
|
||||
const longEmail = `${longLocal}@example.com`;
|
||||
expect(longEmail.length).toBeGreaterThan(254);
|
||||
expect(isValidEmail(longEmail)).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject email that only contains @", (): void => {
|
||||
expect(isValidEmail("just@")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject plaintext without any structure", (): void => {
|
||||
expect(isValidEmail("not-an-email")).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("toWorkspaceMemberRole", (): void => {
|
||||
describe("valid roles", (): void => {
|
||||
it("should return OWNER for 'OWNER'", (): void => {
|
||||
expect(toWorkspaceMemberRole("OWNER")).toBe(WorkspaceMemberRole.OWNER);
|
||||
});
|
||||
|
||||
it("should return ADMIN for 'ADMIN'", (): void => {
|
||||
expect(toWorkspaceMemberRole("ADMIN")).toBe(WorkspaceMemberRole.ADMIN);
|
||||
});
|
||||
|
||||
it("should return MEMBER for 'MEMBER'", (): void => {
|
||||
expect(toWorkspaceMemberRole("MEMBER")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should return GUEST for 'GUEST'", (): void => {
|
||||
expect(toWorkspaceMemberRole("GUEST")).toBe(WorkspaceMemberRole.GUEST);
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid roles", (): void => {
|
||||
it("should fall back to MEMBER for empty string", (): void => {
|
||||
expect(toWorkspaceMemberRole("")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should fall back to MEMBER for unknown role", (): void => {
|
||||
expect(toWorkspaceMemberRole("SUPERADMIN")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should fall back to MEMBER for lowercase variant", (): void => {
|
||||
expect(toWorkspaceMemberRole("admin")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should fall back to MEMBER for mixed case", (): void => {
|
||||
expect(toWorkspaceMemberRole("Admin")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should fall back to MEMBER for numeric input", (): void => {
|
||||
expect(toWorkspaceMemberRole("123")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should fall back to MEMBER for special characters", (): void => {
|
||||
expect(toWorkspaceMemberRole("<script>")).toBe(WorkspaceMemberRole.MEMBER);
|
||||
});
|
||||
});
|
||||
});
|
||||
96
apps/web/src/components/workspace/validation.ts
Normal file
96
apps/web/src/components/workspace/validation.ts
Normal file
@@ -0,0 +1,96 @@
|
||||
import { WorkspaceMemberRole } from "@mosaic/shared";
|
||||
|
||||
/**
|
||||
* Allowed characters in the local part of an email per RFC 5322.
|
||||
* Simple character class with anchors -- no backtracking risk.
|
||||
*/
|
||||
const LOCAL_PART_REGEX = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+$/;
|
||||
|
||||
/**
|
||||
* Checks whether a single character is alphanumeric (a-z, A-Z, 0-9).
|
||||
*/
|
||||
function isAlphanumeric(ch: string): boolean {
|
||||
const code = ch.charCodeAt(0);
|
||||
return (
|
||||
(code >= 48 && code <= 57) || // 0-9
|
||||
(code >= 65 && code <= 90) || // A-Z
|
||||
(code >= 97 && code <= 122) // a-z
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a single domain label per RFC 5321.
|
||||
* - 1 to 63 characters
|
||||
* - Starts and ends with alphanumeric
|
||||
* - Middle characters may include hyphens
|
||||
* Entirely programmatic to avoid regex backtracking concerns.
|
||||
*/
|
||||
function isValidDomainLabel(label: string): boolean {
|
||||
if (label.length === 0 || label.length > 63) {
|
||||
return false;
|
||||
}
|
||||
if (!isAlphanumeric(label.charAt(0))) {
|
||||
return false;
|
||||
}
|
||||
if (label.length > 1 && !isAlphanumeric(label.charAt(label.length - 1))) {
|
||||
return false;
|
||||
}
|
||||
for (let i = 1; i < label.length - 1; i++) {
|
||||
const ch = label.charAt(i);
|
||||
if (!isAlphanumeric(ch) && ch !== "-") {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates an email address using RFC 5322-aligned rules.
|
||||
* Uses programmatic splitting with bounded checks per segment
|
||||
* to avoid ReDoS vulnerabilities from complex single-pass patterns.
|
||||
*/
|
||||
export function isValidEmail(email: string): boolean {
|
||||
if (!email || email.length > 254) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const atIndex = email.indexOf("@");
|
||||
if (atIndex < 1 || atIndex === email.length - 1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Ensure only one @ sign
|
||||
if (email.slice(atIndex + 1).includes("@")) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const localPart = email.slice(0, atIndex);
|
||||
const domain = email.slice(atIndex + 1);
|
||||
|
||||
// Validate local part (max 64 chars per RFC 5321)
|
||||
if (localPart.length > 64 || !LOCAL_PART_REGEX.test(localPart)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Validate domain: split into labels and check each
|
||||
const labels = domain.split(".");
|
||||
if (labels.length < 2) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return labels.every(isValidDomainLabel);
|
||||
}
|
||||
|
||||
const VALID_ROLES = new Set<string>(Object.values(WorkspaceMemberRole));
|
||||
|
||||
/**
|
||||
* Validates a string value against the WorkspaceMemberRole enum.
|
||||
* Returns the validated role if it matches a known enum value,
|
||||
* or falls back to WorkspaceMemberRole.MEMBER if the value is invalid.
|
||||
*/
|
||||
export function toWorkspaceMemberRole(value: string): WorkspaceMemberRole {
|
||||
if (VALID_ROLES.has(value)) {
|
||||
return value as WorkspaceMemberRole;
|
||||
}
|
||||
return WorkspaceMemberRole.MEMBER;
|
||||
}
|
||||
Reference in New Issue
Block a user