From faf7e3c928b683cca6bdf0ca9049079802c98a67 Mon Sep 17 00:00:00 2001
From: Darcy Ye <darcyye@silverhand.io>
Date: Sun, 26 Jan 2025 10:46:52 +0800
Subject: [PATCH] chore(core): hide oidcClientMetadata of SAML apps when using
 GET app APIs (#6979)

---
 .../src/routes/applications/application.ts    | 26 ++++++++++++++++---
 .../core/src/routes/saml-application/index.ts |  1 -
 .../api/application/saml-application.test.ts  | 17 +++++++++---
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/packages/core/src/routes/applications/application.ts b/packages/core/src/routes/applications/application.ts
index 828e54b66d1..3cc40fb7fc8 100644
--- a/packages/core/src/routes/applications/application.ts
+++ b/packages/core/src/routes/applications/application.ts
@@ -1,6 +1,6 @@
 // TODO: @darcyYe refactor this file later to remove disable max line comment
 /* eslint-disable max-lines */
-import type { Role } from '@logto/schemas';
+import type { Role, Application } from '@logto/schemas';
 import {
   Applications,
   ApplicationType,
@@ -13,6 +13,7 @@ import { generateStandardId, generateStandardSecret } from '@logto/shared';
 import { conditional } from '@silverhand/essentials';
 import { boolean, object, string, z } from 'zod';
 
+import { EnvSet } from '#src/env-set/index.js';
 import RequestError from '#src/errors/RequestError/index.js';
 import koaGuard from '#src/middleware/koa-guard.js';
 import koaPagination from '#src/middleware/koa-pagination.js';
@@ -37,6 +38,22 @@ const parseIsThirdPartQueryParam = (isThirdPartyQuery: 'true' | 'false' | undefi
   return isThirdPartyQuery === 'true';
 };
 
+const hideOidcClientMetadataForSamlApp = (application: Application) => {
+  return {
+    ...application,
+    ...conditional(
+      application.type === ApplicationType.SAML &&
+        EnvSet.values.isDevFeaturesEnabled && {
+          oidcClientMetadata: buildOidcClientMetadata(),
+        }
+    ),
+  };
+};
+
+const hideOidcClientMetadataForSamlApps = (applications: readonly Application[]) => {
+  return applications.map((application) => hideOidcClientMetadataForSamlApp(application));
+};
+
 const applicationTypeGuard = z.nativeEnum(ApplicationType);
 
 export default function applicationRoutes<T extends ManagementApiRouter>(
@@ -101,13 +118,14 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
       );
 
       if (paginationDisabled) {
-        ctx.body = await queries.applications.findApplications({
+        const rawApplications = await queries.applications.findApplications({
           search,
           excludeApplicationIds,
           excludeOrganizationId,
           types,
           isThirdParty,
         });
+        ctx.body = hideOidcClientMetadataForSamlApps(rawApplications);
 
         return next();
       }
@@ -134,7 +152,7 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
 
       // Return totalCount to pagination middleware
       ctx.pagination.totalCount = count;
-      ctx.body = applications;
+      ctx.body = hideOidcClientMetadataForSamlApps(applications);
 
       return next();
     }
@@ -238,7 +256,7 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
         await queries.applicationsRoles.findApplicationsRolesByApplicationId(id);
 
       ctx.body = {
-        ...application,
+        ...hideOidcClientMetadataForSamlApp(application),
         isAdmin: includesInternalAdminRole(applicationsRoles),
       };
 
diff --git a/packages/core/src/routes/saml-application/index.ts b/packages/core/src/routes/saml-application/index.ts
index 2ecb692fce9..6ac11c8f35d 100644
--- a/packages/core/src/routes/saml-application/index.ts
+++ b/packages/core/src/routes/saml-application/index.ts
@@ -179,7 +179,6 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
 
   router.post(
     '/saml-applications/:id/secrets',
-    koaQuotaGuard({ key: 'samlApplicationsLimit', quota }),
     koaGuard({
       params: z.object({ id: z.string() }),
       // The life span of the SAML app secret is in years (at least 1 year), and for security concern, secrets which never expire are not recommended.
diff --git a/packages/integration-tests/src/tests/api/application/saml-application.test.ts b/packages/integration-tests/src/tests/api/application/saml-application.test.ts
index 1a846444ac9..54f01e34056 100644
--- a/packages/integration-tests/src/tests/api/application/saml-application.test.ts
+++ b/packages/integration-tests/src/tests/api/application/saml-application.test.ts
@@ -5,6 +5,7 @@ import {
   createApplication,
   deleteApplication,
   getApplications,
+  getApplication,
   updateApplication,
 } from '#src/api/application.js';
 import {
@@ -30,19 +31,29 @@ describe('SAML application', () => {
       description: 'test',
     });
 
+    await expect(getApplication(createdSamlApplication.id)).resolves.toMatchObject(
+      expect.objectContaining({
+        oidcClientMetadata: {
+          redirectUris: [],
+          postLogoutRedirectUris: [],
+        },
+      })
+    );
+
     expect(createdSamlApplication.nameIdFormat).toBe(NameIdFormat.Persistent);
 
     // Check if the SAML application's OIDC metadata redirect URI is properly set.
     // We need to do this since we do not return OIDC related info when using SAML app APIs.
     const samlApplications = await getApplications([ApplicationType.SAML]);
+    expect(samlApplications.every(({ type }) => type === ApplicationType.SAML));
     const pickedSamlApplication = samlApplications.find(
       ({ id }) => id === createdSamlApplication.id
     );
     expect(pickedSamlApplication).toBeDefined();
-    expect(pickedSamlApplication!.oidcClientMetadata.redirectUris.length).toBe(1);
     expect(
-      pickedSamlApplication!.oidcClientMetadata.redirectUris[0]!.endsWith(
-        `api/saml-applications/${createdSamlApplication.id}/callback`
+      samlApplications.every(
+        ({ oidcClientMetadata: { redirectUris, postLogoutRedirectUris } }) =>
+          redirectUris.length === 0 && postLogoutRedirectUris.length === 0
       )
     ).toBe(true);