diff --git a/packages/core/src/libraries/protected-app.test.ts b/packages/core/src/libraries/protected-app.test.ts index e77bee6c4..391417821 100644 --- a/packages/core/src/libraries/protected-app.test.ts +++ b/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 => mockProtectedApplication); -const findApplicationByProtectedAppHost = jest.fn(); const updateApplicationById = jest.fn(async (id: string, data: Partial) => ({ ...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, diff --git a/packages/core/src/libraries/protected-app.ts b/packages/core/src/libraries/protected-app.ts index 5986028f9..f055e0177 100644 --- a/packages/core/src/libraries/protected-app.ts +++ b/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> => { + 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 => { @@ -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> => { - 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, 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 0c786a27a..3ae278449 100644 --- a/packages/core/src/middleware/koa-slonik-error-handler.test.ts +++ b/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, + }) + ); + }); }); diff --git a/packages/core/src/middleware/koa-slonik-error-handler.ts b/packages/core/src/middleware/koa-slonik-error-handler.ts index 809d8d2a2..d3e2ac5fa 100644 --- a/packages/core/src/middleware/koa-slonik-error-handler.ts +++ b/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(): Middleware { 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 }; diff --git a/packages/core/src/queries/application.ts b/packages/core/src/queries/application.ts index 4da7b2175..3a868020d 100644 --- a/packages/core/src/queries/application.ts +++ b/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(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, diff --git a/packages/core/src/routes/applications/application.test.ts b/packages/core/src/routes/applications/application.test.ts index 250d1fe7e..980b7d494 100644 --- a/packages/core/src/routes/applications/application.test.ts +++ b/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), }, } diff --git a/packages/core/src/routes/applications/application.ts b/packages/core/src/routes/applications/application.ts index 5869645bc..ae8eb9936 100644 --- a/packages/core/src/routes/applications/application.ts +++ b/packages/core/src/routes/applications/application.ts @@ -173,7 +173,7 @@ export default function applicationRoutes( ...conditional( rest.type === ApplicationType.Protected && protectedAppMetadata && - (await protectedApps.checkAndBuildProtectedAppData(protectedAppMetadata)) + (await protectedApps.buildProtectedAppData(protectedAppMetadata)) ), ...rest, }); diff --git a/packages/schemas/alterations/next-1706510290-protected-app-host-index.ts b/packages/schemas/alterations/next-1706510290-protected-app-host-index.ts new file mode 100644 index 000000000..97feae7d6 --- /dev/null +++ b/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; diff --git a/packages/schemas/tables/applications.sql b/packages/schemas/tables/applications.sql index 6f6db0c97..ffb7ec618 100644 --- a/packages/schemas/tables/applications.sql +++ b/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') + );