From 7a949b384885f518d6dd62cf803f42649a2d0629 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Mon, 4 Nov 2024 17:22:20 +0800 Subject: [PATCH] chore: add SAML to application type --- .../ApplicationCreation/CreateForm/index.tsx | 2 +- .../src/components/ApplicationIcon/index.tsx | 6 +-- .../console/src/components/Guide/hooks.ts | 4 +- .../ItemPreview/ApplicationPreview.tsx | 5 +- packages/console/src/consts/applications.ts | 3 +- .../GuideDrawer/index.tsx | 24 +++++---- .../ApplicationDetailsContent/index.tsx | 3 +- .../IdpInitiatedAuth/ConfigForm.tsx | 31 ++++++++---- .../src/routes/applications/application.ts | 24 ++++++--- .../application/application.secrets.test.ts | 3 +- .../tests/api/application/application.test.ts | 13 ++++- .../tests/console/applications/constants.ts | 12 +++-- .../src/locales/en/errors/application.ts | 1 + ...xt-1730712629-add-saml-application-type.ts | 50 +++++++++++++++++++ ...-saml-app-third-party-consistency-check.ts | 20 ++++++++ packages/schemas/tables/applications.sql | 7 ++- 16 files changed, 163 insertions(+), 45 deletions(-) create mode 100644 packages/schemas/alterations/next-1730712629-add-saml-application-type.ts create mode 100644 packages/schemas/alterations/next-1730712645-add-saml-app-third-party-consistency-check.ts diff --git a/packages/console/src/components/ApplicationCreation/CreateForm/index.tsx b/packages/console/src/components/ApplicationCreation/CreateForm/index.tsx index 56ee18126..8bf01d668 100644 --- a/packages/console/src/components/ApplicationCreation/CreateForm/index.tsx +++ b/packages/console/src/components/ApplicationCreation/CreateForm/index.tsx @@ -161,7 +161,7 @@ function CreateForm({ > {Object.values(ApplicationType) // Other application types (e.g. "Protected") should not show up in the creation modal - .filter((value) => + .filter((value): value is Exclude => [ ApplicationType.Native, ApplicationType.SPA, diff --git a/packages/console/src/components/ApplicationIcon/index.tsx b/packages/console/src/components/ApplicationIcon/index.tsx index e4c1d7bf1..5a61d0ab1 100644 --- a/packages/console/src/components/ApplicationIcon/index.tsx +++ b/packages/console/src/components/ApplicationIcon/index.tsx @@ -1,5 +1,4 @@ -import type { ApplicationType } from '@logto/schemas'; -import { Theme } from '@logto/schemas'; +import { ApplicationType, Theme } from '@logto/schemas'; import { darkModeApplicationIconMap, @@ -16,7 +15,8 @@ type Props = { }; const getIcon = (type: ApplicationType, isLightMode: boolean, isThirdParty?: boolean) => { - if (isThirdParty) { + // We have ensured that SAML applications are always third party in DB schema, we use `??` here to make TypeScript happy. + if (isThirdParty ?? type === ApplicationType.SAML) { return isLightMode ? thirdPartyApplicationIcon : thirdPartyApplicationIconDark; } diff --git a/packages/console/src/components/Guide/hooks.ts b/packages/console/src/components/Guide/hooks.ts index e73c5e143..3eceb14d2 100644 --- a/packages/console/src/components/Guide/hooks.ts +++ b/packages/console/src/components/Guide/hooks.ts @@ -1,3 +1,4 @@ +import { ApplicationType } from '@logto/schemas'; import { useCallback, useMemo } from 'react'; import { guides } from '@/assets/docs/guides'; @@ -98,7 +99,8 @@ export const useAppGuideMetadata = (): { return accumulated; } - if (isThirdParty) { + // We have ensured that SAML applications are always third party in DB schema, we use `||` here to make TypeScript happy. + if (target === ApplicationType.SAML || isThirdParty) { return { ...accumulated, [thirdPartyAppCategory]: [...accumulated[thirdPartyAppCategory], guide], diff --git a/packages/console/src/components/ItemPreview/ApplicationPreview.tsx b/packages/console/src/components/ItemPreview/ApplicationPreview.tsx index 438d32283..0639df6ee 100644 --- a/packages/console/src/components/ItemPreview/ApplicationPreview.tsx +++ b/packages/console/src/components/ItemPreview/ApplicationPreview.tsx @@ -1,4 +1,4 @@ -import { type Application } from '@logto/schemas'; +import { type Application, ApplicationType } from '@logto/schemas'; import { useTranslation } from 'react-i18next'; import ApplicationIcon from '@/components/ApplicationIcon'; @@ -21,7 +21,8 @@ function ApplicationPreview({ data: { id, name, isThirdParty, type } }: Props) { ]: SvgComponent; }; export const lightModeApplicationIconMap: ApplicationIconMap = Object.freeze({ diff --git a/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/GuideDrawer/index.tsx b/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/GuideDrawer/index.tsx index 95a4bfce1..dbaf731d8 100644 --- a/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/GuideDrawer/index.tsx +++ b/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/GuideDrawer/index.tsx @@ -1,4 +1,4 @@ -import { type ApplicationResponse } from '@logto/schemas'; +import { ApplicationType, type ApplicationResponse } from '@logto/schemas'; import { useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; @@ -26,24 +26,30 @@ function GuideDrawer({ app, secrets, onClose }: Props) { const { getStructuredAppGuideMetadata } = useAppGuideMetadata(); const [selectedGuide, setSelectedGuide] = useState(); + const appType = useMemo( + // SAML application is actually a Traditional application, the same as OIDC applications. + () => (app.type === ApplicationType.SAML ? ApplicationType.Traditional : app.type), + [app.type] + ); + const structuredMetadata = useMemo( - () => getStructuredAppGuideMetadata({ categories: [app.type] }), - [getStructuredAppGuideMetadata, app.type] + () => getStructuredAppGuideMetadata({ categories: [appType] }), + [getStructuredAppGuideMetadata, appType] ); const hasSingleGuide = useMemo(() => { - return structuredMetadata[app.type].length === 1; - }, [app.type, structuredMetadata]); + return structuredMetadata[appType].length === 1; + }, [appType, structuredMetadata]); useEffect(() => { if (hasSingleGuide) { - const guide = structuredMetadata[app.type][0]; + const guide = structuredMetadata[appType][0]; if (guide) { const { id, metadata } = guide; setSelectedGuide({ id, metadata }); } } - }, [hasSingleGuide, app.type, structuredMetadata]); + }, [hasSingleGuide, appType, structuredMetadata]); return (
@@ -75,8 +81,8 @@ function GuideDrawer({ app, secrets, onClose }: Props) { {!selectedGuide && ( { setSelectedGuide(guide); }} diff --git a/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/index.tsx b/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/index.tsx index 102910dce..8804b452d 100644 --- a/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/index.tsx +++ b/packages/console/src/pages/ApplicationDetails/ApplicationDetailsContent/index.tsx @@ -123,7 +123,8 @@ function ApplicationDetailsContent({ data, secrets, oidcConfig, onApplicationUpd icon={} title={data.name} primaryTag={ - data.isThirdParty + // We have ensured that SAML applications are always third party in DB schema, we use `||` here to make TypeScript happy. + data.isThirdParty || data.type === ApplicationType.SAML ? t(`${applicationTypeI18nKey.thirdParty}.title`) : t(`${applicationTypeI18nKey[data.type]}.title`) } diff --git a/packages/console/src/pages/EnterpriseSsoDetails/IdpInitiatedAuth/ConfigForm.tsx b/packages/console/src/pages/EnterpriseSsoDetails/IdpInitiatedAuth/ConfigForm.tsx index 5ed0c7307..0beac2f20 100644 --- a/packages/console/src/pages/EnterpriseSsoDetails/IdpInitiatedAuth/ConfigForm.tsx +++ b/packages/console/src/pages/EnterpriseSsoDetails/IdpInitiatedAuth/ConfigForm.tsx @@ -177,17 +177,28 @@ function ConfigForm({ placeholder={t( 'enterprise_sso_details.idp_initiated_auth_config.empty_applications_placeholder' )} - options={applications.map((application) => ({ - value: application.id, - title: ( - - {application.name} - - ({t(`guide.categories.${application.type}`)}, ID: {application.id}) + options={applications + .filter( + // See definition of `applicationsSearchUrl`, there is only non-third party SPA/Traditional applications here, and SAML applications are always third party secured by DB schema, we need to manually exclude other application types here to make TypeScript happy. + ( + application + ): application is Exclude & { + type: Extract; + } => + application.type === ApplicationType.SPA || + application.type === ApplicationType.Traditional + ) + .map((application) => ({ + value: application.id, + title: ( + + {application.name} + + ({t(`guide.categories.${application.type}`)}, ID: {application.id}) + - - ), - }))} + ), + }))} value={value} error={emptyApplicationsError ?? errors.config?.defaultApplicationId?.message} onChange={onChange} diff --git a/packages/core/src/routes/applications/application.ts b/packages/core/src/routes/applications/application.ts index 2794114b2..14b49aec7 100644 --- a/packages/core/src/routes/applications/application.ts +++ b/packages/core/src/routes/applications/application.ts @@ -1,5 +1,5 @@ // TODO: @darcyYe refactor this file later to remove disable max line comment - +/* eslint-disable max-lines */ import type { Role } from '@logto/schemas'; import { Applications, @@ -147,10 +147,14 @@ export default function applicationRoutes( response: Applications.guard, status: [200, 400, 422, 500], }), - + // eslint-disable-next-line complexity async (ctx, next) => { const { oidcClientMetadata, protectedAppMetadata, ...rest } = ctx.guard.body; + if (rest.type === ApplicationType.SAML) { + throw new RequestError('application.use_saml_app_api'); + } + await Promise.all([ rest.type === ApplicationType.MachineToMachine && quota.guardTenantUsageByKey('machineToMachineLimit'), @@ -262,6 +266,11 @@ export default function applicationRoutes( const { isAdmin, protectedAppMetadata, ...rest } = body; + const pendingUpdateApplication = await queries.applications.findApplicationById(id); + if (pendingUpdateApplication.type === ApplicationType.SAML) { + throw new RequestError('application.use_saml_app_api'); + } + // @deprecated // User can enable the admin access of Machine-to-Machine apps by switching on a toggle on Admin Console. // Since those apps sit in the user tenant, we provide an internal role to apply the necessary scopes. @@ -292,8 +301,7 @@ export default function applicationRoutes( } if (protectedAppMetadata) { - const { type, protectedAppMetadata: originProtectedAppMetadata } = - await queries.applications.findApplicationById(id); + const { type, protectedAppMetadata: originProtectedAppMetadata } = pendingUpdateApplication; assertThat(type === ApplicationType.Protected, 'application.protected_application_only'); assertThat( originProtectedAppMetadata, @@ -319,9 +327,10 @@ export default function applicationRoutes( } } - ctx.body = await (Object.keys(rest).length > 0 - ? queries.applications.updateApplicationById(id, rest, 'replace') - : queries.applications.findApplicationById(id)); + ctx.body = + Object.keys(rest).length > 0 + ? await queries.applications.updateApplicationById(id, rest, 'replace') + : pendingUpdateApplication; return next(); } @@ -359,3 +368,4 @@ export default function applicationRoutes( applicationCustomDataRoutes(router, tenant); } +/* eslint-enable max-lines */ diff --git a/packages/integration-tests/src/tests/api/application/application.secrets.test.ts b/packages/integration-tests/src/tests/api/application/application.secrets.test.ts index 3f4634271..548f246e1 100644 --- a/packages/integration-tests/src/tests/api/application/application.secrets.test.ts +++ b/packages/integration-tests/src/tests/api/application/application.secrets.test.ts @@ -27,7 +27,8 @@ describe('application secrets', () => { await Promise.all(applications.map(async ({ id }) => deleteApplication(id).catch(noop))); }); - it.each(Object.values(ApplicationType))( + // Exclude SAML app since it has different API for operations. + it.each(Object.values(ApplicationType).filter((type) => type !== ApplicationType.SAML))( 'should or not to create application secret for %s applications per type', async (type) => { const application = await createApplication( diff --git a/packages/integration-tests/src/tests/api/application/application.test.ts b/packages/integration-tests/src/tests/api/application/application.test.ts index 6093436f2..bb6324ee7 100644 --- a/packages/integration-tests/src/tests/api/application/application.test.ts +++ b/packages/integration-tests/src/tests/api/application/application.test.ts @@ -26,7 +26,7 @@ describe('application APIs', () => { expect(fetchedApplication.id).toBe(application.id); }); - it('should throw error when creating a third party application with invalid type', async () => { + it('should throw error when creating an OIDC third party application with invalid type', async () => { await expectRejects( createApplication('test-create-app', ApplicationType.Native, { isThirdParty: true, @@ -35,7 +35,16 @@ describe('application APIs', () => { ); }); - it('should create third party application successfully', async () => { + it('should throw error when creating a non-third party SAML application', async () => { + await expectRejects(createApplication('test-create-saml-app', ApplicationType.SAML), { + code: 'application.use_saml_app_api', + status: 400, + }); + }); + + // TODO: add tests for blocking updating SAML application with `PATCH /applications/:id` API, we can not do it before we implement the `POST /saml-applications` API + + it('should create OIDC third party application successfully', async () => { const applicationName = 'test-third-party-app'; const application = await createApplication(applicationName, ApplicationType.Traditional, { diff --git a/packages/integration-tests/src/tests/console/applications/constants.ts b/packages/integration-tests/src/tests/console/applications/constants.ts index b8f22f42e..fc2323e8b 100644 --- a/packages/integration-tests/src/tests/console/applications/constants.ts +++ b/packages/integration-tests/src/tests/console/applications/constants.ts @@ -64,8 +64,10 @@ export type ApplicationMetadata = { description: string; }; -export const applicationTypesMetadata = Object.entries(ApplicationType).map(([key, value]) => ({ - type: value, - name: `${key} app`, - description: `This is a ${key} app`, -})) satisfies ApplicationMetadata[]; +export const applicationTypesMetadata = Object.entries(ApplicationType) + .filter(([_, value]) => value !== ApplicationType.SAML) + .map(([key, value]) => ({ + type: value, + name: `${key} app`, + description: `This is a ${key} app`, + })) satisfies ApplicationMetadata[]; diff --git a/packages/phrases/src/locales/en/errors/application.ts b/packages/phrases/src/locales/en/errors/application.ts index 40a67768e..ca715ecb5 100644 --- a/packages/phrases/src/locales/en/errors/application.ts +++ b/packages/phrases/src/locales/en/errors/application.ts @@ -10,6 +10,7 @@ const application = { protected_app_metadata_is_required: 'Protected app metadata is required.', protected_app_not_configured: 'Protected app provider is not configured. This feature is not available for open source version.', + use_saml_app_api: 'Use `[METHOD] /saml-applications(/.*)?` API to operate SAML app.', cloudflare_unknown_error: 'Got unknown error when requesting Cloudflare API', protected_application_only: 'The feature is only available for protected applications.', protected_application_misconfigured: 'Protected application is misconfigured.', diff --git a/packages/schemas/alterations/next-1730712629-add-saml-application-type.ts b/packages/schemas/alterations/next-1730712629-add-saml-application-type.ts new file mode 100644 index 000000000..add27d03f --- /dev/null +++ b/packages/schemas/alterations/next-1730712629-add-saml-application-type.ts @@ -0,0 +1,50 @@ +import { sql } from '@silverhand/slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +const alteration: AlterationScript = { + up: async (pool) => { + await pool.query(sql` + alter type application_type add value 'SAML'; + `); + }, + down: async (pool) => { + await pool.query(sql` + alter table organization_application_relations drop constraint application_type; + alter table application_secrets drop constraint application_type; + alter table sso_connector_idp_initiated_auth_configs drop constraint application_type; + + drop function check_application_type; + + create type application_type_new as enum ('Native', 'SPA', 'Traditional', 'MachineToMachine', 'Protected'); + delete from applications where "type"='SAML'; + alter table applications + alter column "type" type application_type_new + using ("type"::text::application_type_new); + drop type application_type; + alter type application_type_new rename to application_type; + + create function check_application_type( + application_id varchar(21), + variadic target_type application_type[] + ) returns boolean as + $$ begin + return (select type from applications where id = application_id) = any(target_type); + end; $$ language plpgsql set search_path = public; + + alter table organization_application_relations + add constraint application_type + check (check_application_type(application_id, 'MachineToMachine')); + + alter table application_secrets + add constraint application_type + check (check_application_type(application_id, 'MachineToMachine', 'Traditional', 'Protected')); + + alter table sso_connector_idp_initiated_auth_configs + add constraint application_type + check (check_application_type(default_application_id, 'Traditional', 'SPA')); + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/alterations/next-1730712645-add-saml-app-third-party-consistency-check.ts b/packages/schemas/alterations/next-1730712645-add-saml-app-third-party-consistency-check.ts new file mode 100644 index 000000000..173161dc6 --- /dev/null +++ b/packages/schemas/alterations/next-1730712645-add-saml-app-third-party-consistency-check.ts @@ -0,0 +1,20 @@ +import { sql } from '@silverhand/slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +const alteration: AlterationScript = { + up: async (pool) => { + await pool.query(sql` + alter table applications + add constraint check_saml_app_third_party_consistency + check (type != 'SAML' OR (type = 'SAML' AND is_third_party = true)); + `); + }, + down: async (pool) => { + await pool.query(sql` + alter table applications drop constraint check_saml_app_third_party_consistency; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/tables/applications.sql b/packages/schemas/tables/applications.sql index 6645179f6..98f97ce99 100644 --- a/packages/schemas/tables/applications.sql +++ b/packages/schemas/tables/applications.sql @@ -1,6 +1,6 @@ /* init_order = 1 */ -create type application_type as enum ('Native', 'SPA', 'Traditional', 'MachineToMachine', 'Protected'); +create type application_type as enum ('Native', 'SPA', 'Traditional', 'MachineToMachine', 'Protected', 'SAML'); create table applications ( tenant_id varchar(21) not null @@ -17,7 +17,10 @@ create table applications ( custom_data jsonb /* @use JsonObject */ not null default '{}'::jsonb, is_third_party boolean not null default false, created_at timestamptz not null default(now()), - primary key (id) + primary key (id), + constraint check_saml_app_third_party_consistency check ( + type != 'SAML' OR (type = 'SAML' AND is_third_party = true) + ) ); create index applications__id