diff --git a/controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts b/controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts index c9241da888..69874c892b 100644 --- a/controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts +++ b/controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts @@ -13,7 +13,6 @@ import { SubgraphRepository } from '../../repositories/SubgraphRepository.js'; import type { RouterOptions } from '../../routes.js'; import { enrichLogger, getLogger, handleError, validateDateRanges } from '../../util.js'; import { UnauthorizedError } from '../../errors/errors.js'; -import { federatedGraphs } from '../../../db/schema.js'; export function getChecksByFederatedGraphName( opts: RouterOptions, diff --git a/controlplane/src/core/bufservices/organization/createOrganization.ts b/controlplane/src/core/bufservices/organization/createOrganization.ts index b49540fda6..bf0ece4edd 100644 --- a/controlplane/src/core/bufservices/organization/createOrganization.ts +++ b/controlplane/src/core/bufservices/organization/createOrganization.ts @@ -14,6 +14,7 @@ import type { RouterOptions } from '../../routes.js'; import { BillingService } from '../../services/BillingService.js'; import { enrichLogger, getLogger, handleError } from '../../util.js'; import { OrganizationGroupRepository } from '../../repositories/OrganizationGroupRepository.js'; +import { organizationSchema } from '../../constants.js'; export function createOrganization( opts: RouterOptions, @@ -26,6 +27,17 @@ export function createOrganization( const authContext = await opts.authenticator.authenticate(ctx.requestHeader); logger = enrichLogger(ctx, logger, authContext); + const validatedReq = organizationSchema.safeParse(req); + if (!validatedReq.success) { + const { fieldErrors } = validatedReq.error.flatten(); + return { + response: { + code: EnumStatusCode.ERR_BAD_REQUEST, + details: fieldErrors.name?.[0] || fieldErrors.slug?.[0] || 'Invalid request', + }, + }; + } + const billingRepo = new BillingRepository(opts.db); const plans = await billingRepo.listPlans(); @@ -55,7 +67,7 @@ export function createOrganization( // Create the organization group in Keycloak + subgroups const [kcRootGroupId, kcCreatedGroups] = await opts.keycloakClient.seedGroup({ userID: authContext.userId, - organizationSlug: req.slug, + organizationSlug: validatedReq.data.slug, realm: opts.keycloakRealm, }); @@ -68,8 +80,8 @@ export function createOrganization( const auditLogRepo = new AuditLogRepository(tx); const organization = await orgRepo.createOrganization({ - organizationName: req.name, - organizationSlug: req.slug, + organizationName: validatedReq.data.name, + organizationSlug: validatedReq.data.slug, ownerID: authContext.userId, kcGroupId: kcRootGroupId, }); diff --git a/controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts b/controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts index 5daeafac90..776bce3aef 100644 --- a/controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts +++ b/controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts @@ -8,8 +8,9 @@ import { import { AuditLogRepository } from '../../repositories/AuditLogRepository.js'; import { OrganizationRepository } from '../../repositories/OrganizationRepository.js'; import type { RouterOptions } from '../../routes.js'; -import { enrichLogger, getLogger, handleError, isValidOrganizationName, isValidOrganizationSlug } from '../../util.js'; +import { enrichLogger, getLogger, handleError } from '../../util.js'; import { UnauthorizedError } from '../../errors/errors.js'; +import { organizationSchema } from '../../constants.js'; export function updateOrganizationDetails( opts: RouterOptions, @@ -58,33 +59,25 @@ export function updateOrganizationDetails( throw new UnauthorizedError(); } - if (!isValidOrganizationSlug(req.organizationSlug)) { + const validatedReq = organizationSchema.safeParse({ name: req.organizationName, slug: req.organizationSlug }); + if (!validatedReq.success) { + const { fieldErrors } = validatedReq.error.flatten(); return { response: { - code: EnumStatusCode.ERR, - details: - 'Invalid slug. It must be of 3-24 characters in length, start and end with an alphanumeric character and may contain hyphens in between.', - }, - }; - } - - if (!isValidOrganizationName(req.organizationName)) { - return { - response: { - code: EnumStatusCode.ERR, - details: 'Invalid name. It must be of 1-24 characters in length.', + code: EnumStatusCode.ERR_BAD_REQUEST, + details: fieldErrors.name?.[0] || fieldErrors.slug?.[0] || 'Invalid request', }, }; } - if (org.slug !== req.organizationSlug) { + if (org.slug !== validatedReq.data.slug) { // checking if the provided orgSlug is available - const newOrg = await orgRepo.bySlug(req.organizationSlug); + const newOrg = await orgRepo.bySlug(validatedReq.data.slug); if (newOrg) { return { response: { code: EnumStatusCode.ERR_ALREADY_EXISTS, - details: `Organization with slug ${req.organizationSlug} already exists.`, + details: `Organization with slug ${validatedReq.data.slug} already exists.`, }, }; } @@ -99,7 +92,7 @@ export function updateOrganizationDetails( id: org.kcGroupId, realm: opts.keycloakRealm, }, - { name: req.organizationSlug }, + { name: validatedReq.data.slug }, ); // Rename all the organization roles @@ -113,7 +106,7 @@ export function updateOrganizationDetails( await opts.keycloakClient.client.roles.updateById( { realm: opts.keycloakRealm, id: kcRole.id! }, { - name: kcRole.name!.replace(`${org.slug}:`, `${req.organizationSlug}:`), + name: kcRole.name!.replace(`${org.slug}:`, `${validatedReq.data.slug}:`), }, ); } @@ -121,8 +114,8 @@ export function updateOrganizationDetails( await orgRepo.updateOrganization({ id: authContext.organizationId, - name: req.organizationName, - slug: req.organizationSlug, + name: validatedReq.data.name, + slug: validatedReq.data.slug, }); await auditLogRepo.addAuditLog({ diff --git a/controlplane/src/core/constants.ts b/controlplane/src/core/constants.ts index f94f6ab294..a808741a2c 100644 --- a/controlplane/src/core/constants.ts +++ b/controlplane/src/core/constants.ts @@ -1,3 +1,5 @@ +import * as z from 'zod'; + export const apiKeyPermissions = [ { displayName: 'System for Cross-domain Identity Management (SCIM)', @@ -9,3 +11,32 @@ export const delayForManualOrgDeletionInDays = 3; export const delayForOrgAuditLogsDeletionInDays = 90; export const deafultRangeInHoursForGetOperations = 7 * 24; + +export const organizationSlugSchema = z + .string() + .trim() + .toLowerCase() + .regex( + /^[\da-z]+(?:-[\da-z]+)*$/, + 'Slug should start and end with an alphanumeric character. Spaces and special characters other that hyphen not allowed.', + ) + .min(3, { + message: + 'Invalid slug. It must be of 3-32 characters in length, start and end with an alphanumeric character and may contain hyphens in between.', + }) + .max(32, { + message: + 'Invalid slug. It must be of 3-32 characters in length, start and end with an alphanumeric character and may contain hyphens in between.', + }) + .refine((value) => !['login', 'signup', 'create', 'account'].includes(value), 'This slug is a reserved keyword.'); + +export const organizationSchema = z.object({ + name: z + .string() + .trim() + .min(3, { + message: 'Invalid name. It must be of 3-32 characters in length.', + }) + .max(32, { message: 'Invalid name. It must be of 3-32 characters in length.' }), + slug: organizationSlugSchema, +}); diff --git a/controlplane/src/core/repositories/OrganizationRepository.ts b/controlplane/src/core/repositories/OrganizationRepository.ts index e372a1bc35..a374b860fd 100644 --- a/controlplane/src/core/repositories/OrganizationRepository.ts +++ b/controlplane/src/core/repositories/OrganizationRepository.ts @@ -131,7 +131,7 @@ export class OrganizationRepository { .from(organizations) .leftJoin(organizationBilling, eq(organizations.id, organizationBilling.organizationId)) .leftJoin(billingSubscriptions, eq(organizations.id, billingSubscriptions.organizationId)) - .where(eq(organizations.slug, slug)) + .where(eq(sql`lower(${organizations.slug})`, slug.toLowerCase())) .limit(1) .execute(); diff --git a/controlplane/src/core/util.test.ts b/controlplane/src/core/util.test.ts index 3fd7cc410f..4d91ee1d85 100644 --- a/controlplane/src/core/util.test.ts +++ b/controlplane/src/core/util.test.ts @@ -1,11 +1,6 @@ import { describe, expect, test } from 'vitest'; -import { - extractOperationNames, - hasLabelsChanged, - isValidLabels, - isValidNamespaceName, - isValidOrganizationSlug, -} from './util.js'; +import { extractOperationNames, hasLabelsChanged, isValidLabels, isValidNamespaceName } from './util.js'; +import { organizationSlugSchema } from './constants.js'; describe('Util', (ctx) => { test('Should validate label', () => { @@ -211,15 +206,19 @@ describe('Util', (ctx) => { { slug: 'acme-corp', expected: true }, { slug: '1acme-corp2', expected: true }, { slug: 'ac', expected: false }, - { slug: '25CharactersLong123456789', expected: false }, + { slug: '25CharactersLong123456789', expected: true }, { slug: 'acme-', expected: false }, { slug: '-acme', expected: false }, { slug: 'ac_24', expected: false }, { slug: '1a$c', expected: false }, + { slug: ' ', expected: false }, + { slug: 'a', expected: false }, + { slug: 'a'.repeat(50), expected: false }, ]; for (const entry of slugs) { - expect(isValidOrganizationSlug(entry.slug)).equal(entry.expected); + const parsed = organizationSlugSchema.safeParse(entry.slug); + expect(parsed.success).equal(entry.expected); } }); diff --git a/controlplane/src/core/util.ts b/controlplane/src/core/util.ts index 808a50e1e4..bd196cbc84 100644 --- a/controlplane/src/core/util.ts +++ b/controlplane/src/core/util.ts @@ -37,7 +37,6 @@ import { composeFederatedContract, composeFederatedGraphWithPotentialContracts } import { SubgraphsToCompose } from './repositories/FeatureFlagRepository.js'; const labelRegex = /^[\dA-Za-z](?:[\w.-]{0,61}[\dA-Za-z])?$/; -const organizationSlugRegex = /^[\da-z]+(?:-[\da-z]+)*$/; const namespaceRegex = /^[\da-z]+(?:[_-][\da-z]+)*$/; const schemaTagRegex = /^(?![/-])[\d/A-Za-z-]+(? { return graphNameRegex.test(name); }; -export const isValidOrganizationSlug = (slug: string): boolean => { - // these reserved slugs are the root paths of the studio, - // so the org slug should not be the same as one of our root paths - const reservedSlugs = ['login', 'signup', 'create', 'account']; - - if (slug.length < 3 || slug.length > 24) { - return false; - } - - if (!organizationSlugRegex.test(slug)) { - return false; - } - - if (reservedSlugs.includes(slug)) { - return false; - } - - return true; -}; - -export const isValidOrganizationName = (name: string): boolean => { - if (name.length === 0 || name.length > 24) { - return false; - } - - return true; -}; - export const isValidPluginVersion = (version: string): boolean => { return pluginVersionRegex.test(version); }; diff --git a/controlplane/test/deactivate-org.test.ts b/controlplane/test/deactivate-org.test.ts index 45d97e3526..32f79836bf 100644 --- a/controlplane/test/deactivate-org.test.ts +++ b/controlplane/test/deactivate-org.test.ts @@ -103,7 +103,7 @@ describe('Deactivate Organization', (ctx) => { const mainUserContext = users[TestUser.adminAliceCompanyA]; const orgName = genID(); - await client.createOrganization({ + const x = await client.createOrganization({ name: orgName, slug: orgName, }); diff --git a/controlplane/test/organization/create-organization.test.ts b/controlplane/test/organization/create-organization.test.ts index 07d103739f..6cb73f4c64 100644 --- a/controlplane/test/organization/create-organization.test.ts +++ b/controlplane/test/organization/create-organization.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, test, vi } from 'vitest'; +import { afterAll, beforeAll, describe, expect, test } from 'vitest'; import { EnumStatusCode } from '@wundergraph/cosmo-connect/dist/common/common_pb'; import { SetupTest } from '../test-util.js'; import { afterAllSetup, beforeAllSetup, genID } from '../../src/core/test-util.js'; @@ -39,4 +39,102 @@ describe('Create organization', () => { await server.close(); }); + + test('Should fail when provided a name with only spaces', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: ' ', + slug: genID('org'), + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Invalid name. It must be of 3-32 characters in length.'); + + await server.close(); + }); + + test('Should fail when provided a name that is too short', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: 'aa', + slug: genID('org'), + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Invalid name. It must be of 3-32 characters in length.'); + + await server.close(); + }); + + test('Should fail when provided a name that is too long', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: 'a'.repeat(50), + slug: genID('org'), + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Invalid name. It must be of 3-32 characters in length.'); + + await server.close(); + }); + + test('Should fail when provided a slug with only spaces', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: genID('org'), + slug: ' ', + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Slug should start and end with an alphanumeric character. Spaces and special characters other that hyphen not allowed.'); + + await server.close(); + }); + + test.each(['login', 'create', 'signup'])('Should fail when creating an organization with the reserved slug "%s"', async (slug) => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: genID('org'), + slug, + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('This slug is a reserved keyword.'); + + await server.close(); + }) + + test('Should fail when provided a slug that is too short', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: genID('org'), + slug: 'aa', + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Invalid slug. It must be of 3-32 characters in length, start and end with an alphanumeric character and may contain hyphens in between.'); + + await server.close(); + }); + + test('Should fail when provided a slug that is too long', async () => { + const { client, server } = await SetupTest({ dbname, }); + const createOrganizationResponse = await client.createOrganization({ + name: genID('org'), + slug: 'a'.repeat(50), + plan: 'developer', + }); + + expect(createOrganizationResponse.response?.code).toBe(EnumStatusCode.ERR_BAD_REQUEST); + expect(createOrganizationResponse.response?.details).toBe('Invalid slug. It must be of 3-32 characters in length, start and end with an alphanumeric character and may contain hyphens in between.'); + + await server.close(); + }); }); \ No newline at end of file diff --git a/studio/src/pages/create.tsx b/studio/src/pages/create.tsx index bc92404251..eb84621488 100644 --- a/studio/src/pages/create.tsx +++ b/studio/src/pages/create.tsx @@ -83,12 +83,14 @@ const OrganizationForm = () => { const schema = z.object({ name: z .string() + .trim() .min(3, { message: "Organization name must be a minimum of 3 characters", }) .max(32, { message: "Organization name must be maximum 32 characters" }), slug: z .string() + .trim() .toLowerCase() .regex( new RegExp("^[a-z0-9]+(?:-[a-z0-9]+)*$"), @@ -97,9 +99,9 @@ const OrganizationForm = () => { .min(3, { message: "Organization slug must be a minimum of 3 characters", }) - .max(24, { message: "Organization slug must be maximum 24 characters" }) + .max(32, { message: "Organization slug must be maximum 32 characters" }) .refine( - (value) => !["login", "signup", "create"].includes(value), + (value) => !["login", "signup", "create", "account"].includes(value), "This slug is a reserved keyword", ), plan,