From 5735d9ddfcbfcaff634e623b40d59a8eb26b3199 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Mon, 17 Feb 2025 16:05:21 +0800 Subject: [PATCH] fix(core): fix SAML app redirect URIs sync custom domains logic (#7047) fix: fix SAML app redirect URIs sync custom domains logic --- .../saml-application/saml-applications.ts | 105 ++++++++---------- packages/core/src/routes/domain.test.ts | 2 +- packages/core/src/routes/domain.ts | 26 +++-- 3 files changed, 63 insertions(+), 70 deletions(-) diff --git a/packages/core/src/libraries/saml-application/saml-applications.ts b/packages/core/src/libraries/saml-application/saml-applications.ts index 0b176e66a..a7be4607a 100644 --- a/packages/core/src/libraries/saml-application/saml-applications.ts +++ b/packages/core/src/libraries/saml-application/saml-applications.ts @@ -9,7 +9,7 @@ import { } from '@logto/schemas'; import { SearchJointMode } from '@logto/schemas'; import { generateStandardId, ConsoleLog } from '@logto/shared'; -import { removeUndefinedKeys, pick } from '@silverhand/essentials'; +import { removeUndefinedKeys, pick, deduplicate } from '@silverhand/essentials'; import chalk from 'chalk'; import { EnvSet, getTenantEndpoint } from '#src/env-set/index.js'; @@ -123,19 +123,19 @@ export const createSamlApplicationsLibrary = (queries: Queries) => { * Applies custom domain configuration to SAML application redirect URIs. * * @param currentTenantId - The ID of the current tenant used for constructing default hostname URIs - * @param domain - Current tenant custom domain status, if there is no custom domain, pass `undefined` + * @param domains - Current tenant custom domain status, if there is no custom domain, pass `undefined` * @returns * * @example * // With active domain * const app = { redirectUris: ['https://original.example.com'] }; - * syncCustomDomainToSamlApplicationRedirectUrls('tenant1'， { domain: 'https://custom.domain.com' }); + * syncCustomDomainsToSamlApplicationRedirectUrls('tenant1'， { domain: 'https://custom.domain.com' }); * // redirectUris becomes ['https://original.example.com', 'https://custom.domain.com'] * * @example * // Without active domain * const app = { redirectUris: ['https://original.example.com', 'https://custom.domain.com'] }; - * syncCustomDomainToSamlApplicationRedirectUrls('tenant1'); + * syncCustomDomainsToSamlApplicationRedirectUrls('tenant1'); * // redirectUris becomes ['https://original.example.com'] * * @remarks @@ -145,9 +145,9 @@ export const createSamlApplicationsLibrary = (queries: Queries) => { * Ref: * - SAML application: https://github.com/logto-io/rfcs-internal/pull/5 */ - const syncCustomDomainToSamlApplicationRedirectUrls = async ( + const syncCustomDomainsToSamlApplicationRedirectUrls = async ( currentTenantId: string, - domain?: Domain + domains: Domain[] ) => { // Skip for admin tenant and non-cloud environment. if (currentTenantId === adminTenantId || !EnvSet.values.isCloud) { @@ -161,68 +161,53 @@ export const createSamlApplicationsLibrary = (queries: Queries) => { types: [ApplicationType.SAML], }); - // Apply custom domain to SAML applications' redirect URIs if the custom domain is active. - // eslint-disable-next-line unicorn/prefer-ternary - if (domain?.status === DomainStatus.Active) { - await Promise.all( - samlApplications.map(async (samlApplication) => { - // There is only one default redirect URI for each SAML app. - const defaultRedirectUri = samlApplication.oidcClientMetadata.redirectUris.find((url) => - url.startsWith(getTenantEndpoint(currentTenantId, EnvSet.values).hostname) + const applyCustomDomain = (url: string, domain: Domain): string => { + const parsedUrl = new URL(url); + + // Apply custom domain to SAML applications' redirect URIs if the custom domain is active. + if (domain.status === DomainStatus.Active) { + // eslint-disable-next-line @silverhand/fp/no-mutation + parsedUrl.hostname = domain.domain; + } + + return parsedUrl.toString(); + }; + + await Promise.all( + samlApplications.map(async (samlApplication) => { + const defaultRedirectUri = samlApplication.oidcClientMetadata.redirectUris.find((url) => + url.startsWith(getTenantEndpoint(currentTenantId, EnvSet.values).toString()) + ); + + // Should not happen. + if (!defaultRedirectUri) { + consoleLog.warn( + `Can not apply custom domain to SAML app ${samlApplication.id}, since we can not find default redirect URI.` ); + return; + } - // Should not happen. - if (!defaultRedirectUri) { - consoleLog.warn( - `Can not apply custom domain to SAML app ${samlApplication.id}, since we can not find default redirect URI.` - ); - return; - } + const newRedirectUris = deduplicate([ + defaultRedirectUri, + // If the custom domain is deleted or not active, we should remove all custom domains from the redirect URIs. + ...domains.map((domain) => applyCustomDomain(defaultRedirectUri, domain)), + ]); - const redirectUriWithCustomDomain = new URL(defaultRedirectUri); - // eslint-disable-next-line @silverhand/fp/no-mutation - redirectUriWithCustomDomain.hostname = domain.domain; - - await updateApplicationById(samlApplication.id, { - oidcClientMetadata: { - ...samlApplication.oidcClientMetadata, - redirectUris: [defaultRedirectUri, redirectUriWithCustomDomain.toString()], - }, - }); - }) - ); - } else { - // If the custom domain is deleted or not active, we should remove all custom domains from the redirect URIs. - await Promise.all( - samlApplications.map(async (samlApplication) => { - const redirectUrisWithoutCustomDomains = - // Filter out redirect URIs applied with custom domain. - samlApplication.oidcClientMetadata.redirectUris.filter((url) => - url.startsWith(getTenantEndpoint(currentTenantId, EnvSet.values).hostname) - ); - - if (redirectUrisWithoutCustomDomains.length !== 1) { - consoleLog.warn( - `Can not apply custom domain to SAML app ${samlApplication.id}, the redirect URIs do not match the current tenant's endpoint.` - ); - return; - } - - await updateApplicationById(samlApplication.id, { - oidcClientMetadata: { - ...samlApplication.oidcClientMetadata, - redirectUris: redirectUrisWithoutCustomDomains, - }, - }); - }) - ); - } + const updatedApplication = await updateApplicationById(samlApplication.id, { + oidcClientMetadata: { + ...samlApplication.oidcClientMetadata, + redirectUris: newRedirectUris, + }, + }); + consoleLog.info('updatedApplication', updatedApplication); + }) + ); }; return { createSamlApplicationSecret, findSamlApplicationById, updateSamlApplicationById, - syncCustomDomainToSamlApplicationRedirectUrls, + syncCustomDomainsToSamlApplicationRedirectUrls, }; }; diff --git a/packages/core/src/routes/domain.test.ts b/packages/core/src/routes/domain.test.ts index 397cc6cde..7186c8836 100644 --- a/packages/core/src/routes/domain.test.ts +++ b/packages/core/src/routes/domain.test.ts @@ -36,7 +36,7 @@ const mockLibraries = { }, quota: createMockQuotaLibrary(), samlApplications: { - syncCustomDomainToSamlApplicationRedirectUrls: jest.fn(), + syncCustomDomainsToSamlApplicationRedirectUrls: jest.fn(), }, }; diff --git a/packages/core/src/routes/domain.ts b/packages/core/src/routes/domain.ts index 507ef6589..ac1ec6e7a 100644 --- a/packages/core/src/routes/domain.ts +++ b/packages/core/src/routes/domain.ts @@ -16,7 +16,7 @@ export default function domainRoutes( } = queries; const { domains: { syncDomainStatus, addDomain, deleteDomain }, - samlApplications: { syncCustomDomainToSamlApplicationRedirectUrls }, + samlApplications: { syncCustomDomainsToSamlApplicationRedirectUrls }, } = libraries; router.get( @@ -28,12 +28,8 @@ export default function domainRoutes( domains.map(async (domain) => syncDomainStatus(domain)) ); - await trySafe(async () => - Promise.all( - syncedDomains.map(async (syncedDomain) => - syncCustomDomainToSamlApplicationRedirectUrls(tenantId, syncedDomain) - ) - ) + void trySafe(async () => + syncCustomDomainsToSamlApplicationRedirectUrls(tenantId, [...syncedDomains]) ); ctx.body = syncedDomains.map((domain) => pick(domain, ...domainSelectFields)); @@ -57,7 +53,13 @@ export default function domainRoutes( const domain = await findDomainById(id); const syncedDomain = await syncDomainStatus(domain); - await syncCustomDomainToSamlApplicationRedirectUrls(tenantId, syncedDomain); + void trySafe(async () => { + const domains = await findAllDomains(); + const syncedDomains = await Promise.all( + domains.map(async (domain) => syncDomainStatus(domain)) + ); + await syncCustomDomainsToSamlApplicationRedirectUrls(tenantId, [...syncedDomains]); + }); ctx.body = pick(syncedDomain, ...domainSelectFields); @@ -99,7 +101,13 @@ export default function domainRoutes( const { id } = ctx.guard.params; await deleteDomain(id); - await trySafe(async () => syncCustomDomainToSamlApplicationRedirectUrls(tenantId)); + await trySafe(async () => { + const domains = await findAllDomains(); + const syncedDomains = await Promise.all( + domains.map(async (domain) => syncDomainStatus(domain)) + ); + await syncCustomDomainsToSamlApplicationRedirectUrls(tenantId, [...syncedDomains]); + }); ctx.status = 204;