From a93a39aa1b7dc0e7af45ad327e3c71772a9844d2 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 12 Dec 2023 14:54:49 +0800 Subject: [PATCH] feat(core,schemas): add isThirdParty column to the applications table (#5090) * feat(core,schemas): add isThirdParty column to the applications table add isThirdParty column to the applications table * refactor(core): group the application routes under applications directory (#5091) * refactor(core): group the application routes under applications directory group the application routes under applications directory * refactor(core,schemas): refactor the application api guard refactor the application api guard * fix(schemas): fix application patch guard fix application patch guard * fix(test): fix ut fix ut --- packages/core/src/__mocks__/index.ts | 1 + .../application-role.openapi.json | 0 .../application-role.test.ts | 0 .../{ => applications}/application-role.ts | 2 +- .../application.openapi.json | 0 .../{ => applications}/application.test.ts | 22 +++++++++---- .../routes/{ => applications}/application.ts | 32 ++++++++----------- .../core/src/routes/applications/types.ts | 15 +++++++++ packages/core/src/routes/init.ts | 4 +-- packages/core/src/routes/role.application.ts | 4 +-- ...hird-party-column-to-applications-table.ts | 20 ++++++++++++ packages/schemas/src/seeds/application.ts | 1 + packages/schemas/src/types/application.ts | 15 +++++++++ packages/schemas/tables/applications.sql | 4 +++ 14 files changed, 90 insertions(+), 30 deletions(-) rename packages/core/src/routes/{ => applications}/application-role.openapi.json (100%) rename packages/core/src/routes/{ => applications}/application-role.test.ts (100%) rename packages/core/src/routes/{ => applications}/application-role.ts (98%) rename packages/core/src/routes/{ => applications}/application.openapi.json (100%) rename packages/core/src/routes/{ => applications}/application.test.ts (93%) rename packages/core/src/routes/{ => applications}/application.ts (90%) create mode 100644 packages/core/src/routes/applications/types.ts create mode 100644 packages/schemas/alterations/next-1702274830-add-new-third-party-column-to-applications-table.ts diff --git a/packages/core/src/__mocks__/index.ts b/packages/core/src/__mocks__/index.ts index 56f6f34e4..564cdb340 100644 --- a/packages/core/src/__mocks__/index.ts +++ b/packages/core/src/__mocks__/index.ts @@ -38,6 +38,7 @@ export const mockApplication: Application = { idTokenTtl: 5000, refreshTokenTtl: 6_000_000, }, + isThirdParty: false, createdAt: 1_645_334_775_356, }; diff --git a/packages/core/src/routes/application-role.openapi.json b/packages/core/src/routes/applications/application-role.openapi.json similarity index 100% rename from packages/core/src/routes/application-role.openapi.json rename to packages/core/src/routes/applications/application-role.openapi.json diff --git a/packages/core/src/routes/application-role.test.ts b/packages/core/src/routes/applications/application-role.test.ts similarity index 100% rename from packages/core/src/routes/application-role.test.ts rename to packages/core/src/routes/applications/application-role.test.ts diff --git a/packages/core/src/routes/application-role.ts b/packages/core/src/routes/applications/application-role.ts similarity index 98% rename from packages/core/src/routes/application-role.ts rename to packages/core/src/routes/applications/application-role.ts index 2bcefe935..45b31edb1 100644 --- a/packages/core/src/routes/application-role.ts +++ b/packages/core/src/routes/applications/application-role.ts @@ -10,7 +10,7 @@ import koaRoleRlsErrorHandler from '#src/middleware/koa-role-rls-error-handler.j import assertThat from '#src/utils/assert-that.js'; import { parseSearchParamsForSearch } from '#src/utils/search.js'; -import type { AuthedRouter, RouterInitArgs } from './types.js'; +import type { AuthedRouter, RouterInitArgs } from '../types.js'; export default function applicationRoleRoutes( ...[router, { queries }]: RouterInitArgs diff --git a/packages/core/src/routes/application.openapi.json b/packages/core/src/routes/applications/application.openapi.json similarity index 100% rename from packages/core/src/routes/application.openapi.json rename to packages/core/src/routes/applications/application.openapi.json diff --git a/packages/core/src/routes/application.test.ts b/packages/core/src/routes/applications/application.test.ts similarity index 93% rename from packages/core/src/routes/application.test.ts rename to packages/core/src/routes/applications/application.test.ts index ffbb94adb..8a10e7e57 100644 --- a/packages/core/src/routes/application.test.ts +++ b/packages/core/src/routes/applications/application.test.ts @@ -76,6 +76,7 @@ describe('application route', () => { const response = await applicationRequest .post('/applications') .send({ name, type, description }); + expect(response.status).toEqual(200); expect(response.body).toEqual({ ...mockApplication, @@ -153,19 +154,26 @@ describe('application route', () => { }); it('PATCH /applications/:applicationId expect to throw with invalid properties', async () => { - await expect( - applicationRequest.patch('/applications/doo').send({ type: 'node' }) - ).resolves.toHaveProperty('status', 400); await expect( applicationRequest.patch('/applications/doo').send({ - customClientMetadata: { - ...customClientMetadata, - corsAllowedOrigins: [''], - }, + customClientMetadata: 'test', }) ).resolves.toHaveProperty('status', 400); }); + it('PATCH /applications/:applicationId should not allow update application secret, isThirdParty and type', async () => { + const response = await applicationRequest.patch('/applications/foo').send({ + type: ApplicationType.Native, + isThirdParty: true, + secret: 'test', + }); + + expect(response.status).toEqual(200); + + // Should not update the secret, isThirdParty and type + expect(response.body).toEqual(mockApplication); + }); + it('PATCH /applications/:applicationId should save the formatted URIs as per RFC', async () => { await expect( applicationRequest.patch('/applications/foo').send({ diff --git a/packages/core/src/routes/application.ts b/packages/core/src/routes/applications/application.ts similarity index 90% rename from packages/core/src/routes/application.ts rename to packages/core/src/routes/applications/application.ts index bd3334e78..2d207b4ea 100644 --- a/packages/core/src/routes/application.ts +++ b/packages/core/src/routes/applications/application.ts @@ -2,9 +2,9 @@ import type { Role } from '@logto/schemas'; import { demoAppApplicationId, buildDemoAppDataForTenant, - Applications, InternalRole, ApplicationType, + applicationPatchGuard, } from '@logto/schemas'; import { generateStandardId, generateStandardSecret } from '@logto/shared'; import { boolean, object, string, z } from 'zod'; @@ -16,7 +16,9 @@ import { buildOidcClientMetadata } from '#src/oidc/utils.js'; import assertThat from '#src/utils/assert-that.js'; import { parseSearchParamsForSearch } from '#src/utils/search.js'; -import type { AuthedRouter, RouterInitArgs } from './types.js'; +import type { AuthedRouter, RouterInitArgs } from '../types.js'; + +import { applicationResponseGuard, applicationCreateGuard } from './types.js'; const includesInternalAdminRole = (roles: Readonly>) => roles.some(({ role: { name } }) => name === InternalRole.Admin); @@ -63,7 +65,7 @@ export default function applicationRoutes( .or(applicationTypeGuard.transform((type) => [type])) .optional(), }), - response: z.array(Applications.guard), + response: z.array(applicationResponseGuard), status: 200, }), async (ctx, next) => { @@ -103,11 +105,8 @@ export default function applicationRoutes( router.post( '/applications', koaGuard({ - body: Applications.createGuard - .omit({ id: true, createdAt: true }) - .partial() - .merge(Applications.createGuard.pick({ name: true, type: true })), - response: Applications.guard, + body: applicationCreateGuard, + response: applicationResponseGuard, status: [200, 422], }), async (ctx, next) => { @@ -134,7 +133,7 @@ export default function applicationRoutes( '/applications/:id', koaGuard({ params: object({ id: string().min(1) }), - response: Applications.guard.merge(z.object({ isAdmin: z.boolean() })), + response: applicationResponseGuard.merge(z.object({ isAdmin: z.boolean() })), status: [200, 404], }), async (ctx, next) => { @@ -165,15 +164,12 @@ export default function applicationRoutes( '/applications/:id', koaGuard({ params: object({ id: string().min(1) }), - body: Applications.createGuard - .omit({ id: true, createdAt: true }) - .deepPartial() - .merge( - object({ - isAdmin: boolean().optional(), - }) - ), - response: Applications.guard, + body: applicationPatchGuard.deepPartial().merge( + object({ + isAdmin: boolean().optional(), + }) + ), + response: applicationResponseGuard, status: [200, 404, 422, 500], }), async (ctx, next) => { diff --git a/packages/core/src/routes/applications/types.ts b/packages/core/src/routes/applications/types.ts new file mode 100644 index 000000000..03e29ad39 --- /dev/null +++ b/packages/core/src/routes/applications/types.ts @@ -0,0 +1,15 @@ +import { + Applications, + applicationCreateGuard as originalApplicationCreateGuard, +} from '@logto/schemas'; + +import { EnvSet } from '#src/env-set/index.js'; + +// FIXME: @simeng-li Remove this guard once Logto as IdP is ready +export const applicationResponseGuard = EnvSet.values.isDevFeaturesEnabled + ? Applications.guard + : Applications.guard.omit({ isThirdParty: true }); + +export const applicationCreateGuard = EnvSet.values.isDevFeaturesEnabled + ? originalApplicationCreateGuard + : originalApplicationCreateGuard.omit({ isThirdParty: true }); diff --git a/packages/core/src/routes/init.ts b/packages/core/src/routes/init.ts index 427bc9e66..c0e7243fa 100644 --- a/packages/core/src/routes/init.ts +++ b/packages/core/src/routes/init.ts @@ -11,8 +11,8 @@ import type TenantContext from '#src/tenants/TenantContext.js'; import koaAuth from '../middleware/koa-auth/index.js'; import adminUserRoutes from './admin-user/index.js'; -import applicationRoleRoutes from './application-role.js'; -import applicationRoutes from './application.js'; +import applicationRoleRoutes from './applications/application-role.js'; +import applicationRoutes from './applications/application.js'; import authnRoutes from './authn.js'; import connectorRoutes from './connector/index.js'; import customPhraseRoutes from './custom-phrase.js'; diff --git a/packages/core/src/routes/role.application.ts b/packages/core/src/routes/role.application.ts index 7e78b08d7..c65bd2b44 100644 --- a/packages/core/src/routes/role.application.ts +++ b/packages/core/src/routes/role.application.ts @@ -1,4 +1,3 @@ -import { Applications } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { tryThat } from '@silverhand/essentials'; import { object, string } from 'zod'; @@ -8,6 +7,7 @@ import koaGuard from '#src/middleware/koa-guard.js'; import koaPagination from '#src/middleware/koa-pagination.js'; import { parseSearchParamsForSearch } from '#src/utils/search.js'; +import { applicationResponseGuard } from './applications/types.js'; import type { AuthedRouter, RouterInitArgs } from './types.js'; export default function roleApplicationRoutes( @@ -29,7 +29,7 @@ export default function roleApplicationRoutes( koaPagination(), koaGuard({ params: object({ id: string().min(1) }), - response: Applications.guard.array(), + response: applicationResponseGuard.array(), status: [200, 204, 400, 404], }), async (ctx, next) => { diff --git a/packages/schemas/alterations/next-1702274830-add-new-third-party-column-to-applications-table.ts b/packages/schemas/alterations/next-1702274830-add-new-third-party-column-to-applications-table.ts new file mode 100644 index 000000000..76ee320df --- /dev/null +++ b/packages/schemas/alterations/next-1702274830-add-new-third-party-column-to-applications-table.ts @@ -0,0 +1,20 @@ +import { sql } from 'slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +const alteration: AlterationScript = { + up: async (pool) => { + await pool.query(sql` + alter table applications add is_third_party boolean not null default false; + create index applications__is_third_party on applications (tenant_id, is_third_party); + `); + }, + down: async (pool) => { + await pool.query(sql` + drop index applications__is_third_party; + alter table applications drop is_third_party; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/src/seeds/application.ts b/packages/schemas/src/seeds/application.ts index 00dd56a6d..29792520e 100644 --- a/packages/schemas/src/seeds/application.ts +++ b/packages/schemas/src/seeds/application.ts @@ -27,6 +27,7 @@ export const buildDemoAppDataForTenant = (tenantId: string): Application => ({ type: ApplicationType.SPA, oidcClientMetadata: { redirectUris: [], postLogoutRedirectUris: [] }, customClientMetadata: {}, + isThirdParty: false, createdAt: 0, }); diff --git a/packages/schemas/src/types/application.ts b/packages/schemas/src/types/application.ts index 8095bd226..1e2f795cc 100644 --- a/packages/schemas/src/types/application.ts +++ b/packages/schemas/src/types/application.ts @@ -16,3 +16,18 @@ export const featuredApplicationGuard = Applications.guard.pick({ name: true, type: true, }) satisfies z.ZodType; + +export const applicationCreateGuard = Applications.createGuard + .omit({ + id: true, + createdAt: true, + secret: true, + tenantId: true, + }) + .partial() + .merge(Applications.createGuard.pick({ name: true, type: true })); + +export const applicationPatchGuard = applicationCreateGuard.partial().omit({ + type: true, + isThirdParty: true, +}); diff --git a/packages/schemas/tables/applications.sql b/packages/schemas/tables/applications.sql index d3bbf35b4..101b66bbe 100644 --- a/packages/schemas/tables/applications.sql +++ b/packages/schemas/tables/applications.sql @@ -12,9 +12,13 @@ create table applications ( type application_type not null, oidc_client_metadata jsonb /* @use OidcClientMetadata */ not null, custom_client_metadata jsonb /* @use CustomClientMetadata */ not null default '{}'::jsonb, + is_third_party boolean not null default false, created_at timestamptz not null default(now()), primary key (id) ); create index applications__id on applications (tenant_id, id); + +create index applications__is_third_party + on applications (tenant_id, is_third_party);