Merge pull request #6827 from logto-io/yemq-patch-get-saml-app-by-id-apis

feat(core): add `PATCH/GET /saml-applications/:id` APIs

packages/core/src/saml-applications/libraries/saml-applications.ts

@@ -0,0 +1,98 @@
+import {
+  ApplicationType,
+  type SamlApplicationResponse,
+  type PatchSamlApplication,
+} from '@logto/schemas';
+import { generateStandardId } from '@logto/shared';
+import { removeUndefinedKeys } from '@silverhand/essentials';
+
+import RequestError from '#src/errors/RequestError/index.js';
+import type Queries from '#src/tenants/Queries.js';
+import assertThat from '#src/utils/assert-that.js';
+
+import { ensembleSamlApplication, generateKeyPairAndCertificate } from './utils.js';
+
+export const createSamlApplicationsLibrary = (queries: Queries) => {
+  const {
+    applications: { findApplicationById, updateApplicationById },
+    samlApplicationSecrets: { insertSamlApplicationSecret },
+    samlApplicationConfigs: {
+      findSamlApplicationConfigByApplicationId,
+      updateSamlApplicationConfig,
+    },
+  } = queries;
+
+  const createSamlApplicationSecret = async (
+    applicationId: string,
+    // Set certificate life span to 1 year by default.
+    lifeSpanInDays = 365
+  ) => {
+    const { privateKey, certificate, notAfter } = await generateKeyPairAndCertificate(
+      lifeSpanInDays
+    );
+
+    return insertSamlApplicationSecret({
+      id: generateStandardId(),
+      applicationId,
+      privateKey,
+      certificate,
+      expiresAt: Math.floor(notAfter.getTime() / 1000),
+      active: false,
+    });
+  };
+
+  const findSamlApplicationById = async (id: string): Promise<SamlApplicationResponse> => {
+    const application = await findApplicationById(id);
+    assertThat(
+      application.type === ApplicationType.SAML,
+      new RequestError({
+        code: 'application.saml.saml_application_only',
+        status: 422,
+      })
+    );
+
+    const samlConfig = await findSamlApplicationConfigByApplicationId(application.id);
+
+    return ensembleSamlApplication({ application, samlConfig });
+  };
+
+  const updateSamlApplicationById = async (
+    id: string,
+    patchApplicationObject: PatchSamlApplication
+  ): Promise<SamlApplicationResponse> => {
+    const { config, ...applicationData } = patchApplicationObject;
+    const originalApplication = await findApplicationById(id);
+
+    assertThat(
+      originalApplication.type === ApplicationType.SAML,
+      new RequestError({
+        code: 'application.saml.saml_application_only',
+        status: 422,
+      })
+    );
+
+    const [updatedApplication, upToDateSamlConfig] = await Promise.all([
+      Object.keys(applicationData).length > 0
+        ? updateApplicationById(id, removeUndefinedKeys(applicationData))
+        : originalApplication,
+      config
+        ? updateSamlApplicationConfig({
+            set: config,
+            where: { applicationId: id },
+            jsonbMode: 'replace',
+          })
+        : findSamlApplicationConfigByApplicationId(id),
+    ]);
+
+    return ensembleSamlApplication({
+      application: updatedApplication,
+      samlConfig: upToDateSamlConfig,
+    });
+  };
+
+  return {
+    createSamlApplicationSecret,
+    findSamlApplicationById,
+    updateSamlApplicationById,
+  };
+};

packages/core/src/saml-applications/libraries/secrets.ts

@@ -1,34 +0,0 @@
-import { generateStandardId } from '@logto/shared';
-
-import type Queries from '#src/tenants/Queries.js';
-
-import { generateKeyPairAndCertificate } from './utils.js';
-
-export const createSamlApplicationSecretsLibrary = (queries: Queries) => {
-  const {
-    samlApplicationSecrets: { insertSamlApplicationSecret },
-  } = queries;
-
-  const createSamlApplicationSecret = async (
-    applicationId: string,
-    // Set certificate life span to 1 year by default.
-    lifeSpanInDays = 365
-  ) => {
-    const { privateKey, certificate, notAfter } = await generateKeyPairAndCertificate(
-      lifeSpanInDays
-    );
-
-    return insertSamlApplicationSecret({
-      id: generateStandardId(),
-      applicationId,
-      privateKey,
-      certificate,
-      expiresAt: Math.floor(notAfter.getTime() / 1000),
-      active: false,
-    });
-  };
-
-  return {
-    createSamlApplicationSecret,
-  };
-};

packages/core/src/saml-applications/libraries/utils.ts

@@ -1,8 +1,18 @@
 import crypto from 'node:crypto';
 
+import {
+  type SamlApplicationResponse,
+  type Application,
+  type SamlApplicationConfig,
+  type SamlAcsUrl,
+  BindingType,
+} from '@logto/schemas';
 import { addDays } from 'date-fns';
 import forge from 'node-forge';
 
+import RequestError from '#src/errors/RequestError/index.js';
+import assertThat from '#src/utils/assert-that.js';
+
 export const generateKeyPairAndCertificate = async (lifeSpanInDays = 365) => {
   const keypair = forge.pki.rsa.generateKeyPair({ bits: 4096 });
   return createCertificate(keypair, lifeSpanInDays);
@@ -56,3 +66,36 @@ const createCertificate = (keypair: forge.pki.KeyPair, lifeSpanInDays: number) =
     notAfter,
   };
 };
+
+/**
+ * According to the design, a SAML app will be associated with multiple records from various tables.
+ * Therefore, when complete SAML app data is required, it is necessary to retrieve multiple related records and assemble them into a comprehensive SAML app dataset. This dataset includes:
+ * - A record from the `applications` table with a `type` of `SAML`
+ * - A record from the `saml_application_configs` table
+ */
+export const ensembleSamlApplication = ({
+  application,
+  samlConfig,
+}: {
+  application: Application;
+  samlConfig: Pick<SamlApplicationConfig, 'attributeMapping' | 'entityId' | 'acsUrl'>;
+}): SamlApplicationResponse => {
+  return {
+    ...application,
+    ...samlConfig,
+  };
+};
+
+/**
+ * Only HTTP-POST binding is supported for receiving SAML assertions at the moment.
+ */
+export const validateAcsUrl = (acsUrl: SamlAcsUrl) => {
+  const { binding } = acsUrl;
+  assertThat(
+    binding === BindingType.POST,
+    new RequestError({
+      code: 'application.saml.acs_url_binding_not_supported',
+      status: 422,
+    })
+  );
+};

packages/core/src/saml-applications/queries/configs.ts

@@ -16,7 +16,19 @@ export const createSamlApplicationConfigQueries = (pool: CommonQueryMethods) =>
   const updateSamlApplicationConfig = buildUpdateWhereWithPool(pool)(SamlApplicationConfigs, true);
 
   const findSamlApplicationConfigByApplicationId = async (applicationId: string) =>
-    pool.maybeOne<SamlApplicationConfig>(sql`
+    /**
+     * @remarks
+     * 这里我们使用了 `.one()` 方法 instead of `.maybeOne()` 方法，是因为我们在创建 SAML app 时，直接会创建一条对应的 SAML config 记录。这样在后续我们通过 API 操作 SAML app 的 config 时，不需要额外判断：
+     * 1. 是否需要插入 SAML config（一个 alternative 是，在创建 SAML app 时，不插入一条 SAML config 记录，在 PATCH 时，使用 insert into ... on conflict 的 query 来达到同样的目的）
+     * 2. 在 SAML app 对应的 config 不存在时，GET 方法需要对 null 的 SAML config 进行额外的处理
+     * 根据我们的设计，如果在创建 SAML app 时，就同时创建一条 SAML config 记录，在之后的所有场景中，我们只涉及对这条 DB 记录的 update 操作。在我们的业务场景中，没有手动对 SAML config 记录的 delete 操作。
+     *
+     * Here we use the `.one()` method instead of the `.maybeOne()` method because when creating a SAML app, we directly create a corresponding SAML config record. This means that in subsequent API operations on the SAML app's config, we don't need additional checks:
+     * 1. Whether to insert a SAML config (an alternative approach is not to insert a SAML config record when creating the SAML app, and use an `insert into ... on conflict` query during PATCH to achieve the same result).
+     * 2. When the corresponding config for the SAML app does not exist, the GET method needs to handle the null SAML config additionally.
+     * According to our design, if a SAML config record is created at the same time as the SAML app, in all subsequent scenarios, we only deal with update operations on this DB record. In our business scenario, there is no manual deletion of SAML config records.
+     */
+    pool.one<SamlApplicationConfig>(sql`
       select ${sql.join(Object.values(fields), sql`, `)}
       from ${table}
       where ${fields.applicationId}=${applicationId}

packages/core/src/saml-applications/routes/index.ts

@@ -1,19 +1,22 @@
 import {
   ApplicationType,
   samlApplicationCreateGuard,
+  samlApplicationPatchGuard,
   samlApplicationResponseGuard,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { removeUndefinedKeys } from '@silverhand/essentials';
 import { z } from 'zod';
 
+import RequestError from '#src/errors/RequestError/index.js';
 import koaGuard from '#src/middleware/koa-guard.js';
 import { buildOidcClientMetadata } from '#src/oidc/utils.js';
 import { generateInternalSecret } from '#src/routes/applications/application-secret.js';
 import type { ManagementApiRouter, RouterInitArgs } from '#src/routes/types.js';
-import { ensembleSamlApplication, validateAcsUrl } from '#src/saml-applications/routes/utils.js';
 import assertThat from '#src/utils/assert-that.js';
 
+import { ensembleSamlApplication, validateAcsUrl } from '../libraries/utils.js';
+
 export default function samlApplicationRoutes<T extends ManagementApiRouter>(
   ...[router, { queries, libraries }]: RouterInitArgs<T>
 ) {
@@ -22,7 +25,11 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
     samlApplicationConfigs: { insertSamlApplicationConfig },
   } = queries;
   const {
-    samlApplicationSecrets: { createSamlApplicationSecret },
+    samlApplications: {
+      createSamlApplicationSecret,
+      findSamlApplicationById,
+      updateSamlApplicationById,
+    },
   } = libraries;
 
   router.post(
@@ -30,7 +37,7 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
     koaGuard({
       body: samlApplicationCreateGuard,
       response: samlApplicationResponseGuard,
-      status: [201, 400],
+      status: [201, 400, 422],
     }),
     async (ctx, next) => {
       const { name, description, customData, config } = ctx.guard.body;
@@ -72,17 +79,64 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
     }
   );
 
+  router.get(
+    '/saml-applications/:id',
+    koaGuard({
+      params: z.object({
+        id: z.string(),
+      }),
+      response: samlApplicationResponseGuard,
+      status: [200, 404, 422],
+    }),
+    async (ctx, next) => {
+      const { id } = ctx.guard.params;
+
+      const samlApplication = await findSamlApplicationById(id);
+
+      ctx.status = 200;
+      ctx.body = samlApplication;
+
+      return next();
+    }
+  );
+
+  router.patch(
+    '/saml-applications/:id',
+    koaGuard({
+      params: z.object({ id: z.string() }),
+      body: samlApplicationPatchGuard,
+      response: samlApplicationResponseGuard,
+      status: [200, 404, 422],
+    }),
+    async (ctx, next) => {
+      const { id } = ctx.guard.params;
+
+      const updatedSamlApplication = await updateSamlApplicationById(id, ctx.guard.body);
+
+      ctx.status = 200;
+      ctx.body = updatedSamlApplication;
+
+      return next();
+    }
+  );
+
   router.delete(
     '/saml-applications/:id',
     koaGuard({
       params: z.object({ id: z.string() }),
-      status: [204, 400, 404],
+      status: [204, 422, 404],
     }),
     async (ctx, next) => {
       const { id } = ctx.guard.params;
 
       const { type } = await findApplicationById(id);
-      assertThat(type === ApplicationType.SAML, 'application.saml.saml_application_only');
+      assertThat(
+        type === ApplicationType.SAML,
+        new RequestError({
+          code: 'application.saml.saml_application_only',
+          status: 422,
+        })
+      );
 
       await deleteApplicationById(id);
 

packages/core/src/saml-applications/routes/utils.ts

@@ -1,42 +0,0 @@
-import {
-  type SamlApplicationResponse,
-  type Application,
-  type SamlApplicationConfig,
-  type SamlAcsUrl,
-  BindingType,
-} from '@logto/schemas';
-
-import RequestError from '#src/errors/RequestError/index.js';
-import assertThat from '#src/utils/assert-that.js';
-
-/**
- * According to the design, a SAML app will be associated with multiple records from various tables.
- * Therefore, when complete SAML app data is required, it is necessary to retrieve multiple related records and assemble them into a comprehensive SAML app dataset. This dataset includes:
- * - A record from the `applications` table with a `type` of `SAML`
- * - A record from the `saml_application_configs` table
- */
-export const ensembleSamlApplication = ({
-  application,
-  samlConfig,
-}: {
-  application: Application;
-  samlConfig: Pick<SamlApplicationConfig, 'attributeMapping' | 'entityId' | 'acsUrl'>;
-}): SamlApplicationResponse => {
-  return {
-    ...application,
-    ...samlConfig,
-  };
-};
-
-/**
- * Only HTTP-POST binding is supported for receiving SAML assertions at the moment.
- */
-export const validateAcsUrl = (acsUrl: SamlAcsUrl) => {
-  assertThat(
-    acsUrl.binding === BindingType.POST,
-    new RequestError({
-      code: 'application.saml.acs_url_binding_not_supported',
-      status: 422,
-    })
-  );
-};

packages/core/src/tenants/Libraries.ts

@@ -17,7 +17,7 @@ import { createSocialLibrary } from '#src/libraries/social.js';
 import { createSsoConnectorLibrary } from '#src/libraries/sso-connector.js';
 import { createUserLibrary } from '#src/libraries/user.js';
 import { createVerificationStatusLibrary } from '#src/libraries/verification-status.js';
-import { createSamlApplicationSecretsLibrary } from '#src/saml-applications/libraries/secrets.js';
+import { createSamlApplicationsLibrary } from '#src/saml-applications/libraries/saml-applications.js';
 
 import type Queries from './Queries.js';
 
@@ -38,7 +38,7 @@ export default class Libraries {
   passcodes = createPasscodeLibrary(this.queries, this.connectors);
   applications = createApplicationLibrary(this.queries);
   verificationStatuses = createVerificationStatusLibrary(this.queries);
-  samlApplicationSecrets = createSamlApplicationSecretsLibrary(this.queries);
+  samlApplications = createSamlApplicationsLibrary(this.queries);
   roleScopes = createRoleScopeLibrary(this.queries);
   domains = createDomainLibrary(this.queries);
   protectedApps = createProtectedAppLibrary(this.queries);

packages/integration-tests/src/api/saml-application.ts

@@ -1,4 +1,8 @@
-import { type SamlApplicationResponse, type CreateSamlApplication } from '@logto/schemas';
+import {
+  type SamlApplicationResponse,
+  type CreateSamlApplication,
+  type PatchSamlApplication,
+} from '@logto/schemas';
 
 import { authedAdminApi } from './api.js';
 
@@ -11,3 +15,14 @@ export const createSamlApplication = async (createSamlApplication: CreateSamlApp
 
 export const deleteSamlApplication = async (id: string) =>
   authedAdminApi.delete(`saml-applications/${id}`);
+
+export const updateSamlApplication = async (
+  id: string,
+  patchSamlApplication: PatchSamlApplication
+) =>
+  authedAdminApi
+    .patch(`saml-applications/${id}`, { json: patchSamlApplication })
+    .json<SamlApplicationResponse>();
+
+export const getSamlApplication = async (id: string) =>
+  authedAdminApi.get(`saml-applications/${id}`).json<SamlApplicationResponse>();

packages/integration-tests/src/tests/api/application/saml-application.test.ts

@@ -1,7 +1,12 @@
 import { ApplicationType, BindingType } from '@logto/schemas';
 
 import { createApplication, deleteApplication } from '#src/api/application.js';
-import { createSamlApplication, deleteSamlApplication } from '#src/api/saml-application.js';
+import {
+  createSamlApplication,
+  deleteSamlApplication,
+  updateSamlApplication,
+  getSamlApplication,
+} from '#src/api/saml-application.js';
 import { expectRejects } from '#src/helpers/index.js';
 import { devFeatureTest } from '#src/utils.js';
 
@@ -54,14 +59,47 @@ describe('SAML application', () => {
     await deleteSamlApplication(createdSamlApplication.id);
   });
 
-  it('can not delete non-SAML applications with `DEL /saml-applications/:id` API', async () => {
+  it('should be able to update SAML application and get the updated one', async () => {
+    const createdSamlApplication = await createSamlApplication({
+      name: 'test',
+      description: 'test',
+    });
+
+    const newConfig = {
+      acsUrl: {
+        binding: BindingType.POST,
+        url: 'https://example.logto.io/sso/saml',
+      },
+    };
+    const updatedSamlApplication = await updateSamlApplication(createdSamlApplication.id, {
+      name: 'updated',
+      config: newConfig,
+    });
+    const upToDateSamlApplication = await getSamlApplication(createdSamlApplication.id);
+
+    expect(updatedSamlApplication).toEqual(upToDateSamlApplication);
+    expect(updatedSamlApplication.name).toEqual('updated');
+    expect(updatedSamlApplication.acsUrl).toEqual(newConfig.acsUrl);
+
+    await deleteSamlApplication(updatedSamlApplication.id);
+  });
+
+  it('can not delete/update/get non-SAML applications with `DEL /saml-applications/:id` API', async () => {
     const application = await createApplication('test-non-saml-app', ApplicationType.Traditional, {
       isThirdParty: true,
     });
 
     await expectRejects(deleteSamlApplication(application.id), {
       code: 'application.saml.saml_application_only',
-      status: 400,
+      status: 422,
+    });
+    await expectRejects(updateSamlApplication(application.id, { name: 'updated' }), {
+      code: 'application.saml.saml_application_only',
+      status: 422,
+    });
+    await expectRejects(getSamlApplication(application.id), {
+      code: 'application.saml.saml_application_only',
+      status: 422,
     });
     await deleteApplication(application.id);
   });

packages/schemas/src/foundations/jsonb-types/saml-application-configs.ts

@@ -19,5 +19,5 @@ export type SamlAcsUrl = {
 
 export const samlAcsUrlGuard = z.object({
   binding: z.nativeEnum(BindingType),
-  url: z.string(),
+  url: z.string().url(),
 }) satisfies ToZodObject<SamlAcsUrl>;

packages/schemas/src/types/saml-application.ts

@@ -3,7 +3,7 @@ import { type z } from 'zod';
 import { Applications } from '../db-entries/application.js';
 import { SamlApplicationConfigs } from '../db-entries/saml-application-config.js';
 
-import { applicationCreateGuard } from './application.js';
+import { applicationCreateGuard, applicationPatchGuard } from './application.js';
 
 const samlAppConfigGuard = SamlApplicationConfigs.guard.pick({
   attributeMapping: true,
@@ -24,6 +24,19 @@ export const samlApplicationCreateGuard = applicationCreateGuard
 
 export type CreateSamlApplication = z.infer<typeof samlApplicationCreateGuard>;
 
+export const samlApplicationPatchGuard = applicationPatchGuard
+  .pick({
+    name: true,
+    description: true,
+    customData: true,
+  })
+  .extend({
+    // The reason for encapsulating attributeMapping and spMetadata into an object within the config field is that you cannot provide only one of `attributeMapping` or `spMetadata`. Due to the structure of the `saml_application_configs` table, both must be not null.
+    config: samlAppConfigGuard.partial().optional(),
+  });
+
+export type PatchSamlApplication = z.infer<typeof samlApplicationPatchGuard>;
+
 export const samlApplicationResponseGuard = Applications.guard.merge(
   // Partial to allow the optional fields to be omitted in the response.
   // When starting to create a SAML application, SAML configuration is optional, which can lead to the absence of SAML configuration.