fix: prevent blank subdomain from being saved (#18812)
## Summary Fixes #17941 — Saving a blank subdomain causes a redirect to `.website.com`, effectively breaking the workspace. **Root cause:** Three layers all fail to reject an empty string `""`: 1. **Frontend (`SettingsDomain.tsx`):** `SaveButton` has both `onClick={onSave}` and `type="submit"`. The `onClick` fires first, calling `handleSave()` directly without running Zod validation. So `isDefined("")` returns `true`, the confirmation modal opens, and the blank subdomain is submitted. 2. **Backend DTO (`update-workspace-input.ts`):** The `subdomain` field has `@IsString()` + `@IsOptional()` but no pattern validation, so an empty string passes the DTO layer. 3. **Backend service (`workspace.service.ts:152`):** `if (payload.subdomain && ...)` — empty string is falsy in JS, so it skips `validateSubdomainOrThrow()` entirely and writes `subdomain: ""` to the database. **The crash:** After save, the redirect logic does `"myworkspace.website.com".replace("myworkspace", "")` → `".website.com"`, sending the user to an invalid URL. ## Fix - **Frontend:** Call `form.trigger()` at the start of `handleSave` to run Zod validation regardless of whether the function was invoked via `onClick` or `form.handleSubmit`. Returns early with validation error if invalid. - **Backend DTO:** Add `@Matches(/^[a-z0-9][a-z0-9-]{1,28}[a-z0-9]$/)` to reject invalid subdomains at the request validation layer (defense-in-depth). - **Backend service:** Change `if (payload.subdomain && ...)` to `if (isDefined(payload.subdomain) && ...)` so empty strings route through `validateSubdomainOrThrow()` instead of being silently skipped. ## Test plan - [x] Existing `is-subdomain-valid.util.spec.ts` tests pass (36/36) - [x] TypeScript type checks pass for both `twenty-server` and `twenty-front` - [x] oxlint passes on all changed files - [x] Prettier passes on all changed files - [ ] Manual: Navigate to Settings > Domains, clear the subdomain field, click Save — should show validation error, not redirect --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Charles Bochet <charles@twenty.com> Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com>
This commit is contained in:
@@ -194,6 +194,7 @@ export { isDefined } from './validation/isDefined';
|
||||
export { isEmptyObject } from './validation/isEmptyObject';
|
||||
export { isLabelIdentifierFieldMetadataTypes } from './validation/isLabelIdentifierFieldMetadataTypes';
|
||||
export { isValidLocale } from './validation/isValidLocale';
|
||||
export { isValidTwentySubdomain } from './validation/isValidTwentySubdomain';
|
||||
export { isValidUuid } from './validation/isValidUuid';
|
||||
export { isValidVariable } from './validation/isValidVariable';
|
||||
export { normalizeLocale } from './validation/normalizeLocale';
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
import { isValidTwentySubdomain } from '@/utils/validation/isValidTwentySubdomain';
|
||||
|
||||
describe('isValidTwentySubdomain', () => {
|
||||
describe('valid subdomains', () => {
|
||||
it('should accept standard alphanumeric subdomains', () => {
|
||||
expect(isValidTwentySubdomain('abc')).toBe(true);
|
||||
expect(isValidTwentySubdomain('test123')).toBe(true);
|
||||
expect(isValidTwentySubdomain('company1')).toBe(true);
|
||||
expect(isValidTwentySubdomain('workspace2024')).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept subdomains with hyphens in the middle', () => {
|
||||
expect(isValidTwentySubdomain('my-company')).toBe(true);
|
||||
expect(isValidTwentySubdomain('test-workspace')).toBe(true);
|
||||
expect(isValidTwentySubdomain('multi-word-subdomain')).toBe(true);
|
||||
expect(isValidTwentySubdomain('a-b-c-d-e')).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept minimum length subdomains (3 characters)', () => {
|
||||
expect(isValidTwentySubdomain('abc')).toBe(true);
|
||||
expect(isValidTwentySubdomain('a1b')).toBe(true);
|
||||
expect(isValidTwentySubdomain('a-b')).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept maximum length subdomains (30 characters)', () => {
|
||||
const exactly30 = 'a' + 'b'.repeat(28) + 'c';
|
||||
|
||||
expect(exactly30.length).toBe(30);
|
||||
expect(isValidTwentySubdomain(exactly30)).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept numeric-only subdomains', () => {
|
||||
expect(isValidTwentySubdomain('123')).toBe(true);
|
||||
expect(isValidTwentySubdomain('456789')).toBe(true);
|
||||
expect(isValidTwentySubdomain('1-2-3')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('invalid subdomains', () => {
|
||||
it('should reject empty strings', () => {
|
||||
expect(isValidTwentySubdomain('')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains shorter than 3 characters', () => {
|
||||
expect(isValidTwentySubdomain('a')).toBe(false);
|
||||
expect(isValidTwentySubdomain('ab')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains longer than 30 characters', () => {
|
||||
const tooLong = 'a'.repeat(31);
|
||||
|
||||
expect(isValidTwentySubdomain(tooLong)).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains starting with a hyphen', () => {
|
||||
expect(isValidTwentySubdomain('-test')).toBe(false);
|
||||
expect(isValidTwentySubdomain('-abc')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains ending with a hyphen', () => {
|
||||
expect(isValidTwentySubdomain('test-')).toBe(false);
|
||||
expect(isValidTwentySubdomain('abc-')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains with uppercase letters', () => {
|
||||
expect(isValidTwentySubdomain('Test')).toBe(false);
|
||||
expect(isValidTwentySubdomain('MyCompany')).toBe(false);
|
||||
expect(isValidTwentySubdomain('WORKSPACE')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains with special characters', () => {
|
||||
expect(isValidTwentySubdomain('test@company')).toBe(false);
|
||||
expect(isValidTwentySubdomain('my_workspace')).toBe(false);
|
||||
expect(isValidTwentySubdomain('test.company')).toBe(false);
|
||||
expect(isValidTwentySubdomain('workspace#1')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains with spaces', () => {
|
||||
expect(isValidTwentySubdomain('test company')).toBe(false);
|
||||
expect(isValidTwentySubdomain(' test')).toBe(false);
|
||||
expect(isValidTwentySubdomain('test ')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject subdomains starting with "api-"', () => {
|
||||
expect(isValidTwentySubdomain('api-test')).toBe(false);
|
||||
expect(isValidTwentySubdomain('api-company')).toBe(false);
|
||||
expect(isValidTwentySubdomain('api-123')).toBe(false);
|
||||
});
|
||||
|
||||
it('should accept subdomains containing "api" not as prefix', () => {
|
||||
expect(isValidTwentySubdomain('myapi')).toBe(true);
|
||||
expect(isValidTwentySubdomain('rapid')).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject subdomains with only hyphens', () => {
|
||||
expect(isValidTwentySubdomain('---')).toBe(false);
|
||||
expect(isValidTwentySubdomain('----')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject whitespace-only strings', () => {
|
||||
expect(isValidTwentySubdomain(' ')).toBe(false);
|
||||
expect(isValidTwentySubdomain('\t')).toBe(false);
|
||||
expect(isValidTwentySubdomain('\n')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject unicode characters', () => {
|
||||
expect(isValidTwentySubdomain('café')).toBe(false);
|
||||
expect(isValidTwentySubdomain('tëst')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,6 @@
|
||||
export * from './isDefined';
|
||||
export * from './assertIsDefinedOrThrow';
|
||||
export * from './isValidLocale';
|
||||
export * from './isValidTwentySubdomain';
|
||||
export * from './isValidUuid';
|
||||
export * from './normalizeLocale';
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
import { SUBDOMAIN_PATTERN } from '@/constants/SubdomainPattern';
|
||||
|
||||
export const isValidTwentySubdomain = (subdomain: string): boolean => {
|
||||
return SUBDOMAIN_PATTERN.test(subdomain);
|
||||
};
|
||||
Reference in New Issue
Block a user