Skip to content

Commit

Permalink
chore(core): hide oidcClientMetadata of SAML apps when using GET app …
Browse files Browse the repository at this point in the history
…APIs (#6979)
  • Loading branch information
darcyYe authored Jan 26, 2025
1 parent 67b3aea commit faf7e3c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
26 changes: 22 additions & 4 deletions packages/core/src/routes/applications/application.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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';
Expand All @@ -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>(
Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -238,7 +256,7 @@ export default function applicationRoutes<T extends ManagementApiRouter>(
await queries.applicationsRoles.findApplicationsRolesByApplicationId(id);

ctx.body = {
...application,
...hideOidcClientMetadataForSamlApp(application),
isAdmin: includesInternalAdminRole(applicationsRoles),
};

Expand Down
1 change: 0 additions & 1 deletion packages/core/src/routes/saml-application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
createApplication,
deleteApplication,
getApplications,
getApplication,
updateApplication,
} from '#src/api/application.js';
import {
Expand All @@ -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);

Expand Down

0 comments on commit faf7e3c

Please sign in to comment.