fix(core,schemas): use database index to prevent subdomain conflict (#5326)

packages/core/src/libraries/protected-app.test.ts

@@ -33,14 +33,13 @@ const { MockQueries } = await import('#src/test-utils/tenant.js');
 const { createProtectedAppLibrary } = await import('./protected-app.js');
 
 const findApplicationById = jest.fn(async (): Promise<Application> => mockProtectedApplication);
-const findApplicationByProtectedAppHost = jest.fn();
 const updateApplicationById = jest.fn(async (id: string, data: Partial<Application>) => ({
   ...mockProtectedApplication,
   ...data,
 }));
 const {
   syncAppConfigsToRemote,
-  checkAndBuildProtectedAppData,
+  buildProtectedAppData,
   syncAppCustomDomainStatus,
   getDefaultDomain,
   deleteDomainFromRemote,
@@ -48,7 +47,6 @@ const {
   new MockQueries({
     applications: {
       findApplicationById,
-      findApplicationByProtectedAppHost,
       updateApplicationById,
     },
   })
@@ -140,11 +138,11 @@ describe('syncAppConfigsToRemote()', () => {
   });
 });
 
-describe('checkAndBuildProtectedAppData()', () => {
+describe('buildProtectedAppData()', () => {
   const origin = 'https://example.com';
 
   it('should throw if subdomain is invalid', async () => {
-    await expect(checkAndBuildProtectedAppData({ subDomain: 'a-', origin })).rejects.toThrowError(
+    await expect(buildProtectedAppData({ subDomain: 'a-', origin })).rejects.toThrowError(
       new RequestError({
         code: 'application.invalid_subdomain',
         status: 422,
@@ -152,20 +150,10 @@ describe('checkAndBuildProtectedAppData()', () => {
     );
   });
 
-  it('should throw if subdomain is not available', async () => {
-    findApplicationByProtectedAppHost.mockResolvedValueOnce(mockProtectedApplication);
-    await expect(checkAndBuildProtectedAppData({ subDomain: 'a', origin })).rejects.toThrowError(
-      new RequestError({
-        code: 'application.protected_application_subdomain_exists',
-        status: 422,
-      })
-    );
-  });
-
   it('should return data if subdomain is available', async () => {
     const subDomain = 'a';
     const host = `${subDomain}.${mockProtectedAppConfigProviderConfig.domain}`;
-    await expect(checkAndBuildProtectedAppData({ subDomain, origin })).resolves.toEqual({
+    await expect(buildProtectedAppData({ subDomain, origin })).resolves.toEqual({
       protectedAppMetadata: {
         host,
         origin,

packages/core/src/libraries/protected-app.ts

@@ -61,6 +61,46 @@ const getDefaultDomain = async () => {
   return domain;
 };
 
+/**
+ * Build application data for protected app
+ * check if subdomain is valid
+ * generate host based on subdomain
+ * generate default protectedAppMetadata based on host and origin
+ * generate redirectUris and postLogoutRedirectUris based on host
+ */
+const buildProtectedAppData = async ({
+  subDomain,
+  origin,
+}: {
+  subDomain: string;
+  origin: string;
+}): Promise<Pick<Application, 'protectedAppMetadata' | 'oidcClientMetadata'>> => {
+  assertThat(
+    isValidSubdomain(subDomain),
+    new RequestError({
+      code: 'application.invalid_subdomain',
+      status: 422,
+    })
+  );
+
+  // Skip for integration test, use empty value instead
+  const { domain } = EnvSet.values.isIntegrationTest ? { domain: '' } : await getProviderConfig();
+  const host = `${subDomain}.${domain}`;
+
+  return {
+    protectedAppMetadata: {
+      host,
+      origin,
+      sessionDuration: defaultProtectedAppSessionDuration,
+      pageRules: defaultProtectedAppPageRules,
+    },
+    oidcClientMetadata: {
+      redirectUris: [`https://${host}/${protectedAppSignInCallbackUrl}`],
+      postLogoutRedirectUris: [`https://${host}`],
+    },
+  };
+};
+
 /**
  * Call Cloudflare API to add the domain (custom hostname) to the remote
  * and get the DNS records to be added to the DNS provider
@@ -107,7 +147,7 @@ const deleteDomainFromRemote = async (id: string) => {
 
 export const createProtectedAppLibrary = (queries: Queries) => {
   const {
-    applications: { findApplicationById, findApplicationByProtectedAppHost, updateApplicationById },
+    applications: { findApplicationById, updateApplicationById },
   } = queries;
 
   const syncAppConfigsToRemote = async (applicationId: string): Promise<void> => {
@@ -154,56 +194,6 @@ export const createProtectedAppLibrary = (queries: Queries) => {
     }
   };
 
-  /**
-   * Build application data for protected app
-   * check if subdomain is valid
-   * generate host based on subdomain
-   * generate default protectedAppMetadata based on host and origin
-   * generate redirectUris and postLogoutRedirectUris based on host
-   */
-  const checkAndBuildProtectedAppData = async ({
-    subDomain,
-    origin,
-  }: {
-    subDomain: string;
-    origin: string;
-  }): Promise<Pick<Application, 'protectedAppMetadata' | 'oidcClientMetadata'>> => {
-    assertThat(
-      isValidSubdomain(subDomain),
-      new RequestError({
-        code: 'application.invalid_subdomain',
-        status: 422,
-      })
-    );
-
-    // Skip for integration test, use empty value instead
-    const { domain } = EnvSet.values.isIntegrationTest ? { domain: '' } : await getProviderConfig();
-    const host = `${subDomain}.${domain}`;
-
-    const application = await findApplicationByProtectedAppHost(host);
-
-    assertThat(
-      !application,
-      new RequestError({
-        code: 'application.protected_application_subdomain_exists',
-        status: 422,
-      })
-    );
-
-    return {
-      protectedAppMetadata: {
-        host,
-        origin,
-        sessionDuration: defaultProtectedAppSessionDuration,
-        pageRules: defaultProtectedAppPageRules,
-      },
-      oidcClientMetadata: {
-        redirectUris: [`https://${host}/${protectedAppSignInCallbackUrl}`],
-        postLogoutRedirectUris: [`https://${host}`],
-      },
-    };
-  };
-
   /**
    * Query domain status from Cloudflare and update the data and status in the database
    */
@@ -280,7 +270,7 @@ export const createProtectedAppLibrary = (queries: Queries) => {
   return {
     syncAppConfigsToRemote,
     deleteRemoteAppConfigs,
-    checkAndBuildProtectedAppData,
+    buildProtectedAppData,
     getDefaultDomain,
     addDomainToRemote,
     syncAppCustomDomainStatus,

packages/core/src/middleware/koa-slonik-error-handler.test.ts

@@ -1,5 +1,5 @@
 import { Users } from '@logto/schemas';
-import { NotFoundError, SlonikError } from 'slonik';
+import { NotFoundError, SlonikError, UniqueIntegrityConstraintViolationError } from 'slonik';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import { DeletionError, InsertionError, UpdateError } from '#src/errors/SlonikError/index.js';
@@ -98,4 +98,21 @@ describe('koaSlonikErrorHandler middleware', () => {
       })
     );
   });
+
+  it('UniqueIntegrityConstraintViolationError for protected application', async () => {
+    const error = new UniqueIntegrityConstraintViolationError(
+      new Error(' '),
+      'applications__protected_app_metadata_host'
+    );
+    next.mockImplementationOnce(() => {
+      throw error;
+    });
+
+    await expect(koaSlonikErrorHandler()(ctx, next)).rejects.toMatchError(
+      new RequestError({
+        code: 'application.protected_application_subdomain_exists',
+        status: 422,
+      })
+    );
+  });
 });

packages/core/src/middleware/koa-slonik-error-handler.ts

@@ -5,6 +5,7 @@ import {
   NotFoundError,
   InvalidInputError,
   CheckIntegrityConstraintViolationError,
+  UniqueIntegrityConstraintViolationError,
 } from 'slonik';
 
 import RequestError from '#src/errors/RequestError/index.js';
@@ -26,6 +27,16 @@ export default function koaSlonikErrorHandler<StateT, ContextT>(): Middleware<St
         });
       }
 
+      if (
+        error instanceof UniqueIntegrityConstraintViolationError &&
+        error.constraint === 'applications__protected_app_metadata_host'
+      ) {
+        throw new RequestError({
+          code: 'application.protected_application_subdomain_exists',
+          status: 422,
+        });
+      }
+
       if (error instanceof CheckIntegrityConstraintViolationError) {
         throw new RequestError({
           code: 'entity.db_constraint_violated',

packages/core/src/queries/application.test.ts

@@ -22,7 +22,6 @@ const { createApplicationQueries } = await import('./application.js');
 const {
   findTotalNumberOfApplications,
   findApplicationById,
-  findApplicationByProtectedAppHost,
   findApplicationByProtectedAppCustomDomain,
   insertApplication,
   updateApplicationById,
@@ -68,27 +67,6 @@ describe('application query', () => {
     await findApplicationById(id);
   });
 
-  it('findApplicationByProtectedAppHost', async () => {
-    const host = 'host.protected.app';
-    const rowData = { host };
-
-    const expectSql = sql`
-      select ${sql.join(Object.values(fields), sql`, `)}
-      from ${table}
-      where ${fields.protectedAppMetadata}->>'host' = $1
-      and ${fields.type} = $2
-    `;
-
-    mockQuery.mockImplementationOnce(async (sql, values) => {
-      expectSqlAssert(sql, expectSql.sql);
-      expect(values).toEqual([host, ApplicationType.Protected]);
-
-      return createMockQueryResult([rowData]);
-    });
-
-    await findApplicationByProtectedAppHost(host);
-  });
-
   it('findApplicationByProtectedAppCustomDomain', async () => {
     const domain = 'my.blog.com';
     const rowData = { domain };

packages/core/src/queries/application.ts

@@ -130,14 +130,6 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => {
 
   const findApplicationById = buildFindEntityByIdWithPool(pool)(Applications);
 
-  const findApplicationByProtectedAppHost = async (host: string) =>
-    pool.maybeOne<Application>(sql`
-      select ${sql.join(Object.values(fields), sql`, `)}
-      from ${table}
-      where ${fields.protectedAppMetadata}->>'host' = ${host}
-      and ${fields.type} = ${ApplicationType.Protected}
-    `);
-
   /**
    * Find an protected application by its custom domain.
    * the domain is stored in the `customDomains` field of the `protectedAppMetadata` field.
@@ -252,7 +244,6 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => {
     findApplications,
     findTotalNumberOfApplications,
     findApplicationById,
-    findApplicationByProtectedAppHost,
     findApplicationByProtectedAppCustomDomain,
     insertApplication,
     updateApplication,

packages/core/src/routes/applications/application.test.ts

@@ -19,7 +19,7 @@ const findApplicationById = jest.fn(async () => mockApplication);
 const deleteApplicationById = jest.fn();
 const syncAppConfigsToRemote = jest.fn();
 const deleteRemoteAppConfigs = jest.fn();
-const checkAndBuildProtectedAppData = jest.fn(async () => {
+const buildProtectedAppData = jest.fn(async () => {
   const { oidcClientMetadata, protectedAppMetadata } = mockProtectedApplication;
 
   return { oidcClientMetadata, protectedAppMetadata };
@@ -60,7 +60,7 @@ const tenantContext = new MockTenant(
     protectedApps: {
       syncAppConfigsToRemote,
       deleteRemoteAppConfigs,
-      checkAndBuildProtectedAppData,
+      buildProtectedAppData,
       getDefaultDomain: jest.fn(async () => mockProtectedAppConfigProviderConfig.domain),
     },
   }

packages/core/src/routes/applications/application.ts

@@ -173,7 +173,7 @@ export default function applicationRoutes<T extends AuthedRouter>(
         ...conditional(
           rest.type === ApplicationType.Protected &&
             protectedAppMetadata &&
-            (await protectedApps.checkAndBuildProtectedAppData(protectedAppMetadata))
+            (await protectedApps.buildProtectedAppData(protectedAppMetadata))
         ),
         ...rest,
       });

packages/schemas/alterations/next-1706510290-protected-app-host-index.ts

@@ -0,0 +1,21 @@
+import { sql } from 'slonik';
+
+import type { AlterationScript } from '../lib/types/alteration.js';
+
+const alteration: AlterationScript = {
+  up: async (pool) => {
+    await pool.query(sql`
+      create unique index applications__protected_app_metadata_host
+      on applications (
+        (protected_app_metadata->>'host')
+      );
+    `);
+  },
+  down: async (pool) => {
+    await pool.query(sql`
+      drop index applications__protected_app_metadata_host;
+    `);
+  },
+};
+
+export default alteration;

packages/schemas/tables/applications.sql

@@ -23,3 +23,8 @@ create index applications__id
 
 create index applications__is_third_party
   on applications (tenant_id, is_third_party);
+
+create unique index applications__protected_app_metadata_host
+  on applications (
+    (protected_app_metadata->>'host')
+  );