From db2d10a2a809ab7d192fda9e72e195f030a0b881 Mon Sep 17 00:00:00 2001 From: wangsijie Date: Wed, 31 Jan 2024 11:05:20 +0800 Subject: [PATCH] fix(core,schemas): use database index to prevent custom domain conflict (#5342) --- .../koa-slonik-error-handler.test.ts | 19 +++++++++++++- .../middleware/koa-slonik-error-handler.ts | 24 ++++++++++++------ packages/core/src/queries/application.test.ts | 25 +------------------ packages/core/src/queries/application.ts | 14 ----------- ...application-protected-app-metadata.test.ts | 17 +------------ .../application-protected-app-metadata.ts | 16 +++--------- ...5206-protected-app-custom-domain-unique.ts | 21 ++++++++++++++++ packages/schemas/tables/applications.sql | 5 ++++ 8 files changed, 65 insertions(+), 76 deletions(-) create mode 100644 packages/schemas/alterations/next-1706585206-protected-app-custom-domain-unique.ts diff --git a/packages/core/src/middleware/koa-slonik-error-handler.test.ts b/packages/core/src/middleware/koa-slonik-error-handler.test.ts index 3ae278449..e843e1e83 100644 --- a/packages/core/src/middleware/koa-slonik-error-handler.test.ts +++ b/packages/core/src/middleware/koa-slonik-error-handler.test.ts @@ -99,7 +99,7 @@ describe('koaSlonikErrorHandler middleware', () => { ); }); - it('UniqueIntegrityConstraintViolationError for protected application', async () => { + it('UniqueIntegrityConstraintViolationError for protected application host', async () => { const error = new UniqueIntegrityConstraintViolationError( new Error(' '), 'applications__protected_app_metadata_host' @@ -115,4 +115,21 @@ describe('koaSlonikErrorHandler middleware', () => { }) ); }); + + it('UniqueIntegrityConstraintViolationError for protected application custom domain', async () => { + const error = new UniqueIntegrityConstraintViolationError( + new Error(' '), + 'applications__protected_app_metadata_custom_domain' + ); + next.mockImplementationOnce(() => { + throw error; + }); + + await expect(koaSlonikErrorHandler()(ctx, next)).rejects.toMatchError( + new RequestError({ + code: 'domain.hostname_already_exists', + status: 422, + }) + ); + }); }); diff --git a/packages/core/src/middleware/koa-slonik-error-handler.ts b/packages/core/src/middleware/koa-slonik-error-handler.ts index d3e2ac5fa..d5f528165 100644 --- a/packages/core/src/middleware/koa-slonik-error-handler.ts +++ b/packages/core/src/middleware/koa-slonik-error-handler.ts @@ -11,6 +11,7 @@ import { import RequestError from '#src/errors/RequestError/index.js'; import { DeletionError, InsertionError, UpdateError } from '#src/errors/SlonikError/index.js'; +/* eslint-disable complexity */ export default function koaSlonikErrorHandler(): Middleware { return async (ctx, next) => { try { @@ -27,14 +28,20 @@ export default function koaSlonikErrorHandler(): Middleware(): Middleware { await findApplicationById(id); }); - it('findApplicationByProtectedAppCustomDomain', async () => { - const domain = 'my.blog.com'; - const rowData = { domain }; - - const expectSql = sql` - select ${sql.join(Object.values(fields), sql`, `)} - from ${table} - where ${fields.protectedAppMetadata} ? 'customDomains' - and ${fields.protectedAppMetadata}->'customDomains' @> $1::jsonb - and ${fields.type} = $2 - `; - - mockQuery.mockImplementationOnce(async (sql, values) => { - expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([JSON.stringify([domain]), ApplicationType.Protected]); - - return createMockQueryResult([rowData]); - }); - - await findApplicationByProtectedAppCustomDomain(domain); - }); - it('insertApplication', async () => { const keys = excludeAutoSetFields(Applications.fieldKeys); diff --git a/packages/core/src/queries/application.ts b/packages/core/src/queries/application.ts index 3a868020d..80dd295a1 100644 --- a/packages/core/src/queries/application.ts +++ b/packages/core/src/queries/application.ts @@ -130,19 +130,6 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => { const findApplicationById = buildFindEntityByIdWithPool(pool)(Applications); - /** - * Find an protected application by its custom domain. - * the domain is stored in the `customDomains` field of the `protectedAppMetadata` field. - */ - const findApplicationByProtectedAppCustomDomain = async (domain: string) => - pool.maybeOne(sql` - select ${sql.join(Object.values(fields), sql`, `)} - from ${table} - where ${fields.protectedAppMetadata} ? 'customDomains' - and ${fields.protectedAppMetadata}->'customDomains' @> ${sql.jsonb([domain])} - and ${fields.type} = ${ApplicationType.Protected} - `); - const insertApplication = buildInsertIntoWithPool(pool)(Applications, { returning: true, }); @@ -244,7 +231,6 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => { findApplications, findTotalNumberOfApplications, findApplicationById, - findApplicationByProtectedAppCustomDomain, insertApplication, updateApplication, updateApplicationById, diff --git a/packages/core/src/routes/applications/application-protected-app-metadata.test.ts b/packages/core/src/routes/applications/application-protected-app-metadata.test.ts index 6b444f8e1..75506a0ab 100644 --- a/packages/core/src/routes/applications/application-protected-app-metadata.test.ts +++ b/packages/core/src/routes/applications/application-protected-app-metadata.test.ts @@ -1,6 +1,5 @@ -import { type Application, DomainStatus } from '@logto/schemas'; +import { DomainStatus } from '@logto/schemas'; import { pickDefault } from '@logto/shared/esm'; -import { type Nullable } from '@silverhand/essentials'; import { mockCloudflareData, mockProtectedApplication } from '#src/__mocks__/index.js'; import { mockIdGenerators } from '#src/test-utils/nanoid.js'; @@ -13,9 +12,6 @@ const protectedAppSignInCallbackUrl = 'sign-in-callback'; const updateApplicationById = jest.fn(); const findApplicationById = jest.fn(async () => mockProtectedApplication); -const findApplicationByProtectedAppCustomDomain = jest.fn( - async (): Promise> => null -); const mockDomainResponse = { domain: mockDomain, @@ -50,7 +46,6 @@ const tenantContext = new MockTenant( applications: { findApplicationById, updateApplicationById, - findApplicationByProtectedAppCustomDomain, }, }, undefined, @@ -133,16 +128,6 @@ describe('application protected app metadata routes', () => { }); expect(response.status).toEqual(400); }); - - it('throw when the domain is already in use', async () => { - findApplicationByProtectedAppCustomDomain.mockResolvedValueOnce(mockProtectedApplication); - const response = await requester - .post(`/applications/asdf/protected-app-metadata/custom-domains`) - .send({ - domain: mockDomain, - }); - expect(response.status).toEqual(422); - }); }); describe('DELETE /applications/:applicationId/protected-app-metadata/custom-domains/:domain', () => { diff --git a/packages/core/src/routes/applications/application-protected-app-metadata.ts b/packages/core/src/routes/applications/application-protected-app-metadata.ts index ee5464cce..f1fea3a77 100644 --- a/packages/core/src/routes/applications/application-protected-app-metadata.ts +++ b/packages/core/src/routes/applications/application-protected-app-metadata.ts @@ -13,11 +13,7 @@ export default function applicationProtectedAppMetadataRoutes { + await pool.query(sql` + create unique index applications__protected_app_metadata_custom_domain + on applications ( + (protected_app_metadata->'customDomains'->0->>'domain') + ); + `); + }, + down: async (pool) => { + await pool.query(sql` + drop index applications__protected_app_metadata_custom_domain; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/tables/applications.sql b/packages/schemas/tables/applications.sql index ffb7ec618..702f26971 100644 --- a/packages/schemas/tables/applications.sql +++ b/packages/schemas/tables/applications.sql @@ -28,3 +28,8 @@ create unique index applications__protected_app_metadata_host on applications ( (protected_app_metadata->>'host') ); + +create unique index applications__protected_app_metadata_custom_domain + on applications ( + (protected_app_metadata->'customDomains'->0->>'domain') + );