From bd723dccce0bb977650d5e9d1248e4c4f91c2d4b Mon Sep 17 00:00:00 2001 From: Giancarlo Buomprisco Date: Tue, 11 Mar 2025 09:58:21 +0700 Subject: [PATCH] Validate special chars when creating a team (#209) * Add validation for team account names - Prevent creating teams with reserved names like 'billing' and 'settings' - Add regex validation to block team names with special characters - Update localization for new error messages - Extend E2E tests to cover various invalid team name scenarios * Enhance team account name validation and slug generation - Add comprehensive tests for account slug generation in Supabase - Improve team name validation schema to handle special characters - Add form validation message display in update team account name form - Refine slug generation to handle various edge cases like special characters, non-ASCII text, and mixed case --- .../tests/team-accounts/team-accounts.po.ts | 36 ++++--- .../tests/team-accounts/team-accounts.spec.ts | 93 +++++++++++++++++++ apps/web/public/locales/en/teams.json | 3 +- .../tests/database/account-slug.test.sql | 72 ++++++++++++++ .../update-team-account-name-form.tsx | 3 + .../src/schema/create-team.schema.ts | 11 +++ 6 files changed, 205 insertions(+), 13 deletions(-) diff --git a/apps/e2e/tests/team-accounts/team-accounts.po.ts b/apps/e2e/tests/team-accounts/team-accounts.po.ts index 1d71030ae..761238337 100644 --- a/apps/e2e/tests/team-accounts/team-accounts.po.ts +++ b/apps/e2e/tests/team-accounts/team-accounts.po.ts @@ -16,7 +16,7 @@ export class TeamAccountsPageObject { async setup(params = this.createTeamName()) { const { email } = await this.auth.signUpFlow('/home'); - + await this.createTeam(params); return { email, teamName: params.teamName, slug: params.slug }; @@ -78,6 +78,16 @@ export class TeamAccountsPageObject { }).toPass(); } + async tryCreateTeam(teamName: string) { + await this.page.locator('[data-test="create-team-form"] input').fill(''); + await this.page.waitForTimeout(200); + await this.page.locator('[data-test="create-team-form"] input').fill(teamName); + + return this.page.click( + '[data-test="create-team-form"] button:last-child', + ); + } + async createTeam({ teamName, slug } = this.createTeamName()) { await this.openAccountsSelector(); @@ -132,20 +142,20 @@ export class TeamAccountsPageObject { // Find the member row and click the actions button const memberRow = this.page.getByRole('row', { name: memberEmail }); await memberRow.getByRole('button').click(); - + // Click the update role option in the dropdown menu await this.page.getByText('Update Role').click(); - + // Select the new role await this.page.click('[data-test="role-selector-trigger"]'); await this.page.click(`[data-test="role-option-${newRole}"]`); - + // Click the confirm button const click = this.page.click('[data-test="confirm-update-member-role"]'); - + // Wait for the update to complete and page to reload const response = this.page.waitForURL('**/home/*/members'); - + return Promise.all([click, response]); }).toPass(); } @@ -155,19 +165,21 @@ export class TeamAccountsPageObject { // Find the member row and click the actions button const memberRow = this.page.getByRole('row', { name: memberEmail }); await memberRow.getByRole('button').click(); - + // Click the transfer ownership option in the dropdown menu await this.page.getByText('Transfer Ownership').click(); - + // Complete OTP verification await this.otp.completeOtpVerification(ownerEmail); - + // Click the confirm button - const click = this.page.click('[data-test="confirm-transfer-ownership-button"]'); - + const click = this.page.click( + '[data-test="confirm-transfer-ownership-button"]', + ); + // Wait for the transfer to complete and page to reload const response = this.page.waitForURL('**/home/*/members'); - + return Promise.all([click, response]); }).toPass(); } diff --git a/apps/e2e/tests/team-accounts/team-accounts.spec.ts b/apps/e2e/tests/team-accounts/team-accounts.spec.ts index 282712c7c..b5918dcf5 100644 --- a/apps/e2e/tests/team-accounts/team-accounts.spec.ts +++ b/apps/e2e/tests/team-accounts/team-accounts.spec.ts @@ -96,6 +96,99 @@ test.describe('Team Accounts', () => { await expect(teamAccounts.getTeamFromSelector(teamName)).toBeVisible(); }); + + test('cannot create a Team account using reserved names', async ({ + page, + }) => { + const teamAccounts = new TeamAccountsPageObject(page); + await teamAccounts.setup(); + + await teamAccounts.openAccountsSelector(); + await page.click('[data-test="create-team-account-trigger"]'); + + await teamAccounts.tryCreateTeam('billing'); + + await expect( + page.getByText('This name is reserved. Please choose a different one.'), + ).toBeVisible(); + + await teamAccounts.tryCreateTeam('settings'); + + await expect( + page.getByText('This name is reserved. Please choose a different one.'), + ).toBeVisible(); + + function expectError() { + return expect( + page.getByText( + 'This name cannot contain special characters. Please choose a different one.', + ), + ).toBeVisible(); + } + + await teamAccounts.tryCreateTeam('Test-Name#'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test,Name'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name/') + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name\\') + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name:') + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name;') + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name='); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name>'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name<'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name?'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name@'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name^'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name&'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name*'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name('); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name)'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name+'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name%'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name$'); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name['); + await expectError(); + + await teamAccounts.tryCreateTeam('Test Name]'); + await expectError(); + }); }); test.describe('Team Account Deletion', () => { diff --git a/apps/web/public/locales/en/teams.json b/apps/web/public/locales/en/teams.json index 695ecc850..2a0b61413 100644 --- a/apps/web/public/locales/en/teams.json +++ b/apps/web/public/locales/en/teams.json @@ -158,5 +158,6 @@ "joiningTeam": "Joining team...", "leaveTeamInputLabel": "Please type LEAVE to confirm leaving the team.", "leaveTeamInputDescription": "By leaving the team, you will no longer have access to it.", - "reservedNameError": "This name is reserved. Please choose a different one." + "reservedNameError": "This name is reserved. Please choose a different one.", + "specialCharactersError": "This name cannot contain special characters. Please choose a different one." } diff --git a/apps/web/supabase/tests/database/account-slug.test.sql b/apps/web/supabase/tests/database/account-slug.test.sql index 0cbc3c57c..1035619ba 100644 --- a/apps/web/supabase/tests/database/account-slug.test.sql +++ b/apps/web/supabase/tests/database/account-slug.test.sql @@ -51,6 +51,78 @@ select throws_ok( 'duplicate key value violates unique constraint "accounts_slug_key"' ); +-- Test special characters in the slug +update public.accounts set slug = 'test-5' where slug = 'test-4['; + +select row_eq( + $$ select slug from public.accounts where name = 'Test 4' $$, + row('test-4'::text), + 'Updating the name of a team account should update the slug' +); + +-- Test various special characters +update public.accounts set name = 'Test@Special#Chars$' where slug = 'test-4'; + +select row_eq( + $$ select slug from public.accounts where name = 'Test@Special#Chars$' $$, + row('test-special-chars'::text), + 'Special characters should be removed from slug' +); + +-- Test multiple consecutive special characters +update public.accounts set name = 'Test!!Multiple---Special$$$Chars' where slug = 'test-special-chars'; + +select row_eq( + $a$ select slug from public.accounts where name = 'Test!!Multiple---Special$$$Chars' $a$, + row('test-multiple-special-chars'::text), + 'Multiple consecutive special characters should be replaced with single hyphen' +); + +-- Test leading and trailing special characters +update public.accounts set name = '!!!LeadingAndTrailing###' where slug = 'test-multiple-special-chars'; + +select row_eq( + $$ select slug from public.accounts where name = '!!!LeadingAndTrailing###' $$, + row('leadingandtrailing'::text), + 'Leading and trailing special characters should be removed' +); + +-- Test non-ASCII characters +update public.accounts set name = 'Testéñ中文Русский' where slug = 'leadingandtrailing'; + +select row_eq( + $$ select slug from public.accounts where name = 'Testéñ中文Русский' $$, + row('testen'::text), + 'Non-ASCII characters should be transliterated or removed' +); + +-- Test mixed case with special characters +update public.accounts set name = 'Test Mixed CASE With Special@Chars!' where slug = 'testen'; + +select row_eq( + $$ select slug from public.accounts where name = 'Test Mixed CASE With Special@Chars!' $$, + row('test-mixed-case-with-special-chars'::text), + 'Mixed case should be converted to lowercase and special chars handled' +); + +-- Test using parentheses +update public.accounts set name = 'Test (Parentheses)' where slug = 'test-mixed-case-with-special-chars'; + +select row_eq( + $$ select slug from public.accounts where name = 'Test (Parentheses)' $$, + row('test-parentheses'::text), + 'Parentheses should be removed from slug' +); + +-- Test using asterisk +update public.accounts set name = 'Test * Asterisk' where slug = 'test-parentheses'; + +select row_eq( + $$ select slug from public.accounts where name = 'Test * Asterisk' $$, + row('test-asterisk'::text), + 'Asterisk should be removed from slug' +); + select * from finish(); ROLLBACK; \ No newline at end of file diff --git a/packages/features/team-accounts/src/components/settings/update-team-account-name-form.tsx b/packages/features/team-accounts/src/components/settings/update-team-account-name-form.tsx index cf71fff5f..c11268a7f 100644 --- a/packages/features/team-accounts/src/components/settings/update-team-account-name-form.tsx +++ b/packages/features/team-accounts/src/components/settings/update-team-account-name-form.tsx @@ -16,6 +16,7 @@ import { FormField, FormItem, FormLabel, + FormMessage, } from '@kit/ui/form'; import { Input } from '@kit/ui/input'; import { Trans } from '@kit/ui/trans'; @@ -98,6 +99,8 @@ export const UpdateTeamAccountNameForm = (props: { {...field} /> + + ); }} diff --git a/packages/features/team-accounts/src/schema/create-team.schema.ts b/packages/features/team-accounts/src/schema/create-team.schema.ts index 60c5109b1..feadea7c5 100644 --- a/packages/features/team-accounts/src/schema/create-team.schema.ts +++ b/packages/features/team-accounts/src/schema/create-team.schema.ts @@ -12,6 +12,8 @@ const RESERVED_NAMES_ARRAY = [ // please add more reserved names here ]; +const SPECIAL_CHARACTERS_REGEX = /[!@#$%^&*()+=[\]{};':"\\|,.<>/?]/; + /** * @name TeamNameSchema */ @@ -21,6 +23,15 @@ export const TeamNameSchema = z }) .min(2) .max(50) + .refine( + (name) => { + console.log(name); + return !SPECIAL_CHARACTERS_REGEX.test(name); + }, + { + message: 'teams:specialCharactersError', + }, + ) .refine( (name) => { return !RESERVED_NAMES_ARRAY.includes(name.toLowerCase());