Skip to content

Commit

Permalink
chore: add SAML to application type
Browse files Browse the repository at this point in the history
  • Loading branch information
darcyYe committed Nov 8, 2024
1 parent 9f16574 commit 71193e0
Show file tree
Hide file tree
Showing 16 changed files with 163 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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, ApplicationType.SAML> =>
[
ApplicationType.Native,
ApplicationType.SPA,
Expand Down
6 changes: 3 additions & 3 deletions packages/console/src/components/ApplicationIcon/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { ApplicationType } from '@logto/schemas';
import { Theme } from '@logto/schemas';
import { ApplicationType, Theme } from '@logto/schemas';

import {
darkModeApplicationIconMap,
Expand All @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/console/src/components/Guide/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ApplicationType } from '@logto/schemas';
import { useCallback, useMemo } from 'react';

import { guides } from '@/assets/docs/guides';
Expand Down Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -21,7 +21,8 @@ function ApplicationPreview({ data: { id, name, isThirdParty, type } }: Props) {
<ItemPreview
title={name}
subtitle={
isThirdParty
// We have ensured that SAML applications are always third party in DB schema, we use `||` here to make TypeScript happy.
isThirdParty || type === ApplicationType.SAML
? t(`${applicationTypeI18nKey.thirdParty}.title`)
: t(`${applicationTypeI18nKey[type]}.title`)
}
Expand Down
3 changes: 2 additions & 1 deletion packages/console/src/consts/applications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import TraditionalWebAppDark from '@/assets/icons/traditional-web-app-dark.svg?r
import TraditionalWebApp from '@/assets/icons/traditional-web-app.svg?react';

type ApplicationIconMap = {
[key in ApplicationType]: SvgComponent;
// TODO: Add SAML icon when we support SAML application in console

Check warning on line 15 in packages/console/src/consts/applications.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/console/src/consts/applications.ts#L15

[no-warning-comments] Unexpected 'todo' comment: 'TODO: Add SAML icon when we support SAML...'.
[key in Exclude<ApplicationType, ApplicationType.SAML>]: SvgComponent;
};

export const lightModeApplicationIconMap: ApplicationIconMap = Object.freeze({
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -26,24 +26,30 @@ function GuideDrawer({ app, secrets, onClose }: Props) {
const { getStructuredAppGuideMetadata } = useAppGuideMetadata();
const [selectedGuide, setSelectedGuide] = useState<SelectedGuide>();

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 (
<div className={styles.drawerContainer}>
Expand Down Expand Up @@ -75,8 +81,8 @@ function GuideDrawer({ app, secrets, onClose }: Props) {
{!selectedGuide && (
<GuideCardGroup
className={styles.cardGroup}
categoryName={t(`categories.${app.type}`)}
guides={structuredMetadata[app.type]}
categoryName={t(`categories.${appType}`)}
guides={structuredMetadata[appType]}
onClickGuide={(guide) => {
setSelectedGuide(guide);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ function ApplicationDetailsContent({ data, secrets, oidcConfig, onApplicationUpd
icon={<ApplicationIcon type={data.type} isThirdParty={data.isThirdParty} />}
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`)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: (
<span>
{application.name}
<span className={styles.applicationDetails}>
({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<Application, 'type'> & {
type: Extract<ApplicationType, 'SPA' | 'Traditional'>;
} =>
application.type === ApplicationType.SPA ||
application.type === ApplicationType.Traditional
)
.map((application) => ({
value: application.id,
title: (
<span>
{application.name}
<span className={styles.applicationDetails}>
({t(`guide.categories.${application.type}`)}, ID: {application.id})
</span>
</span>
</span>
),
}))}
),
}))}
value={value}
error={emptyApplicationsError ?? errors.config?.defaultApplicationId?.message}
onChange={onChange}
Expand Down
24 changes: 17 additions & 7 deletions packages/core/src/routes/applications/application.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// TODO: @darcyYe refactor this file later to remove disable max line comment

Check warning on line 1 in packages/core/src/routes/applications/application.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: @darcyYe refactor this file later...'.

/* eslint-disable max-lines */
import type { Role } from '@logto/schemas';
import {
Applications,
Expand Down Expand Up @@ -147,10 +147,14 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
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');
}

Check warning on line 156 in packages/core/src/routes/applications/application.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/applications/application.ts#L155-L156

Added lines #L155 - L156 were not covered by tests

await Promise.all([
rest.type === ApplicationType.MachineToMachine &&
quota.guardTenantUsageByKey('machineToMachineLimit'),
Expand Down Expand Up @@ -262,6 +266,11 @@ export default function applicationRoutes<T extends ManagementApiRouter>(

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');
}

Check warning on line 272 in packages/core/src/routes/applications/application.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/applications/application.ts#L271-L272

Added lines #L271 - L272 were not covered by tests

// @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.
Expand Down Expand Up @@ -292,8 +301,7 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
}

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,
Expand All @@ -319,9 +327,10 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
}
}

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();
}
Expand Down Expand Up @@ -359,3 +368,4 @@ export default function applicationRoutes<T extends ManagementApiRouter>(

applicationCustomDataRoutes(router, tenant);
}
/* eslint-enable max-lines */
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Check warning on line 45 in packages/integration-tests/src/tests/api/application/application.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/integration-tests/src/tests/api/application/application.test.ts#L45

[no-warning-comments] Unexpected 'todo' comment: 'TODO: add tests for blocking updating...'.

it('should create OIDC third party application successfully', async () => {
const applicationName = 'test-third-party-app';

const application = await createApplication(applicationName, ApplicationType.Traditional, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
1 change: 1 addition & 0 deletions packages/phrases/src/locales/en/errors/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 5 additions & 2 deletions packages/schemas/tables/applications.sql
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down

0 comments on commit 71193e0

Please sign in to comment.