feat: improve organization creation and update validation

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,

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,
         });

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,16 +106,16 @@ 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}:`),
           },
         );
       }
     }
 
     await orgRepo.updateOrganization({
       id: authContext.organizationId,
-      name: req.organizationName,
-      slug: req.organizationSlug,
+      name: validatedReq.data.name,
+      slug: validatedReq.data.slug,
     });
 
     await auditLogRepo.addAuditLog({

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,
+});

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();
 

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);
     }
   });
 

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-]+(?<![/-])$/;
 const graphNameRegex = /^[\dA-Za-z]+(?:[./@_-][\dA-Za-z]+)*$/;
@@ -312,34 +311,6 @@ export const isValidGraphName = (name: string): boolean => {
   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);
 };

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,
     });

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();
+  });
 });