diff --git a/.gitignore b/.gitignore index a95d6581e..761882df1 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ dump.rdb # console auto generated files /packages/console/src/consts/jwt-customizer-type-definition.ts + +# Temporarily ignore SAML OpenAPI spec file (will be needed later) +packages/core/src/saml-applications/routes/index.openapi.json diff --git a/packages/core/src/routes/init.ts b/packages/core/src/routes/init.ts index fcd49d4f8..48c0283f0 100644 --- a/packages/core/src/routes/init.ts +++ b/packages/core/src/routes/init.ts @@ -100,7 +100,11 @@ const createRouters = (tenant: TenantContext) => { systemRoutes(managementRouter, tenant); subjectTokenRoutes(managementRouter, tenant); accountCentersRoutes(managementRouter, tenant); - if (EnvSet.values.isDevFeaturesEnabled) { + // TODO: @darcy per our design, we will move related routes to Cloud repo and the routes will be loaded from remote. + if ( + (EnvSet.values.isDevFeaturesEnabled && EnvSet.values.isCloud) || + EnvSet.values.isIntegrationTest + ) { samlApplicationRoutes(managementRouter, tenant); } diff --git a/packages/core/src/routes/swagger/utils/operation.ts b/packages/core/src/routes/swagger/utils/operation.ts index 899c7283d..0a3e2b3e9 100644 --- a/packages/core/src/routes/swagger/utils/operation.ts +++ b/packages/core/src/routes/swagger/utils/operation.ts @@ -133,14 +133,14 @@ export const buildRouterObjects = (routers: T[], option router.stack // Filter out universal routes (mostly like a proxy route to withtyped) .filter(({ path }) => !path.includes('.*')) + // TODO: Remove this and bring back `/saml-applications` routes before release. + // Exclude `/saml-applications` routes for now. + .filter(({ path }) => !path.startsWith('/saml-applications')) .flatMap(({ path: routerPath, stack, methods }) => methods .map((method) => method.toLowerCase()) // There is no need to show the HEAD method. .filter((method): method is OpenAPIV3.HttpMethods => method !== 'head') - // TODO: Remove this and bring back `/saml-applications` routes before release. - // Exclude `/saml-applications` routes for now. - .filter(() => !routerPath.startsWith('/saml-applications')) .map((httpMethod) => { const path = normalizePath(routerPath); const operation = buildOperation(httpMethod, stack, routerPath, isAuthGuarded); diff --git a/packages/core/src/saml-applications/libraries/secrets.ts b/packages/core/src/saml-applications/libraries/secrets.ts index 2c93cd089..a18378388 100644 --- a/packages/core/src/saml-applications/libraries/secrets.ts +++ b/packages/core/src/saml-applications/libraries/secrets.ts @@ -9,7 +9,7 @@ export const createSamlApplicationSecretsLibrary = (queries: Queries) => { samlApplicationSecrets: { insertSamlApplicationSecret }, } = queries; - const createNewSamlApplicationSecretForApplication = async ( + const createSamlApplicationSecret = async ( applicationId: string, // Set certificate life span to 1 year by default. lifeSpanInDays = 365 @@ -29,6 +29,6 @@ export const createSamlApplicationSecretsLibrary = (queries: Queries) => { }; return { - createNewSamlApplicationSecretForApplication, + createSamlApplicationSecret, }; }; diff --git a/packages/core/src/saml-applications/libraries/utils.test.ts b/packages/core/src/saml-applications/libraries/utils.test.ts new file mode 100644 index 000000000..90e25e6af --- /dev/null +++ b/packages/core/src/saml-applications/libraries/utils.test.ts @@ -0,0 +1,59 @@ +import { addDays } from 'date-fns'; +import forge from 'node-forge'; + +import { generateKeyPairAndCertificate } from './utils.js'; + +describe('generateKeyPairAndCertificate', () => { + it('should generate valid key pair and certificate', async () => { + const result = await generateKeyPairAndCertificate(); + + // Verify private key format + expect(result.privateKey).toContain('-----BEGIN RSA PRIVATE KEY-----'); + expect(result.privateKey).toContain('-----END RSA PRIVATE KEY-----'); + + // Verify certificate format + expect(result.certificate).toContain('-----BEGIN CERTIFICATE-----'); + expect(result.certificate).toContain('-----END CERTIFICATE-----'); + + // Verify expiration date (default 365 days) + const expectedNotAfter = addDays(new Date(), 365); + expect(result.notAfter.getDate()).toBe(expectedNotAfter.getDate()); + expect(result.notAfter.getMonth()).toBe(expectedNotAfter.getMonth()); + expect(result.notAfter.getFullYear()).toBe(expectedNotAfter.getFullYear()); + + // Verify certificate content + const cert = forge.pki.certificateFromPem(result.certificate); + expect(cert.subject.getField('CN').value).toBe('example.com'); + expect(cert.issuer.getField('CN').value).toBe('logto.io'); + expect(cert.issuer.getField('O').value).toBe('Logto'); + expect(cert.issuer.getField('C').value).toBe('US'); + }); + + it('should generate certificate with custom lifespan', async () => { + const customDays = 30; + const result = await generateKeyPairAndCertificate(customDays); + + const expectedNotAfter = addDays(new Date(), customDays); + expect(result.notAfter.getDate()).toBe(expectedNotAfter.getDate()); + expect(result.notAfter.getMonth()).toBe(expectedNotAfter.getMonth()); + expect(result.notAfter.getFullYear()).toBe(expectedNotAfter.getFullYear()); + }); + + it('should generate unique serial numbers for different certificates', async () => { + const result1 = await generateKeyPairAndCertificate(); + const result2 = await generateKeyPairAndCertificate(); + + const cert1 = forge.pki.certificateFromPem(result1.certificate); + const cert2 = forge.pki.certificateFromPem(result2.certificate); + + expect(cert1.serialNumber).not.toBe(cert2.serialNumber); + }); + + it('should generate RSA key pair with 4096 bits', async () => { + const result = await generateKeyPairAndCertificate(); + const privateKey = forge.pki.privateKeyFromPem(result.privateKey); + + // RSA key should be 4096 bits + expect(forge.pki.privateKeyToPem(privateKey).length).toBeGreaterThan(3000); // A 4096-bit RSA private key in PEM format is typically longer than 3000 characters + }); +}); diff --git a/packages/core/src/saml-applications/routes/index.openapi.json b/packages/core/src/saml-applications/routes/index.openapi.json deleted file mode 100644 index 321455907..000000000 --- a/packages/core/src/saml-applications/routes/index.openapi.json +++ /dev/null @@ -1,95 +0,0 @@ -{ - "tags": [ - { - "name": "SAML applications", - "description": "SAML applications enable Single Sign-On (SSO) integration between Logto (acting as Identity Provider/IdP) and third-party Service Providers (SP) using the SAML 2.0 protocol. These endpoints allow you to manage SAML application configurations." - }, - { - "name": "Dev feature" - } - ], - "paths": { - "/api/saml-applications": { - "post": { - "summary": "Create SAML application", - "description": "Create a new SAML application with the given configuration. This will create both the application entity and its SAML-specific configurations.", - "requestBody": { - "required": true, - "content": { - "application/json": { - "schema": { - "properties": { - "name": { - "type": "string", - "description": "The name of the SAML application." - }, - "description": { - "type": "string", - "description": "The description of the SAML application." - }, - "customData": { - "type": "object", - "description": "Custom data for the application." - }, - "config": { - "type": "object", - "properties": { - "attributeMapping": { - "type": "object", - "description": "Mapping of SAML attributes to Logto user properties." - }, - "entityId": { - "type": "string", - "description": "Service provider's entityId." - }, - "acsUrl": { - "type": "object", - "description": "Service provider assertion consumer service URL configuration." - } - } - } - } - } - } - } - }, - "responses": { - "201": { - "description": "The SAML application was created successfully." - }, - "400": { - "description": "Invalid request body or SAML configuration." - } - } - } - }, - "/api/saml-applications/{id}": { - "delete": { - "summary": "Delete SAML application", - "description": "Delete a SAML application by ID. This will remove both the application entity and its SAML-specific configurations.", - "parameters": [ - { - "name": "id", - "in": "path", - "required": true, - "schema": { - "type": "string" - }, - "description": "The ID of the SAML application to delete." - } - ], - "responses": { - "204": { - "description": "The SAML application was deleted successfully." - }, - "400": { - "description": "Invalid application ID, the application is not a SAML application." - }, - "404": { - "description": "The SAML application was not found." - } - } - } - } - } -} diff --git a/packages/core/src/saml-applications/routes/index.ts b/packages/core/src/saml-applications/routes/index.ts index 2e402a5c1..1d0486614 100644 --- a/packages/core/src/saml-applications/routes/index.ts +++ b/packages/core/src/saml-applications/routes/index.ts @@ -1,6 +1,5 @@ import { ApplicationType, - BindingType, samlApplicationCreateGuard, samlApplicationResponseGuard, } from '@logto/schemas'; @@ -8,12 +7,11 @@ 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 } from '#src/saml-applications/routes/utils.js'; +import { ensembleSamlApplication, validateAcsUrl } from '#src/saml-applications/routes/utils.js'; import assertThat from '#src/utils/assert-that.js'; export default function samlApplicationRoutes( @@ -24,7 +22,7 @@ export default function samlApplicationRoutes( samlApplicationConfigs: { insertSamlApplicationConfig }, } = queries; const { - samlApplicationSecrets: { createNewSamlApplicationSecretForApplication }, + samlApplicationSecrets: { createSamlApplicationSecret }, } = libraries; router.post( @@ -37,12 +35,8 @@ export default function samlApplicationRoutes( async (ctx, next) => { const { name, description, customData, config } = ctx.guard.body; - // Only HTTP-POST binding is supported for receiving SAML assertions at the moment. - if (config?.acsUrl?.binding && config.acsUrl.binding !== BindingType.POST) { - throw new RequestError({ - code: 'application.saml.acs_url_binding_not_supported', - status: 422, - }); + if (config?.acsUrl) { + validateAcsUrl(config.acsUrl); } const application = await insertApplication( @@ -58,16 +52,21 @@ export default function samlApplicationRoutes( }) ); - const [samlConfig, samlSecret] = await Promise.all([ - insertSamlApplicationConfig({ - applicationId: application.id, - ...config, - }), - createNewSamlApplicationSecretForApplication(application.id), - ]); + try { + const [samlConfig, _] = await Promise.all([ + insertSamlApplicationConfig({ + applicationId: application.id, + ...config, + }), + createSamlApplicationSecret(application.id), + ]); - ctx.status = 201; - ctx.body = ensembleSamlApplication({ application, samlConfig, samlSecret }); + ctx.status = 201; + ctx.body = ensembleSamlApplication({ application, samlConfig }); + } catch (error) { + await deleteApplicationById(application.id); + throw error; + } return next(); } @@ -82,11 +81,8 @@ export default function samlApplicationRoutes( async (ctx, next) => { const { id } = ctx.guard.params; - const { type, isThirdParty } = await findApplicationById(id); - assertThat( - type === ApplicationType.SAML && isThirdParty, - 'application.saml.saml_application_only' - ); + const { type } = await findApplicationById(id); + assertThat(type === ApplicationType.SAML, 'application.saml.saml_application_only'); await deleteApplicationById(id); diff --git a/packages/core/src/saml-applications/routes/utils.ts b/packages/core/src/saml-applications/routes/utils.ts index 55ac46852..857a1378e 100644 --- a/packages/core/src/saml-applications/routes/utils.ts +++ b/packages/core/src/saml-applications/routes/utils.ts @@ -1,22 +1,42 @@ import { type SamlApplicationResponse, - type SamlApplicationSecret, 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, - samlSecret, }: { application: Application; samlConfig: Pick; - samlSecret?: SamlApplicationSecret | SamlApplicationSecret[]; }): SamlApplicationResponse => { return { ...application, ...samlConfig, - secrets: samlSecret ? (Array.isArray(samlSecret) ? samlSecret : [samlSecret]) : [], }; }; + +/** + * 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, + }) + ); +}; 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 ef581839d..e990dd28e 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 @@ -14,18 +14,6 @@ describe('SAML application', () => { description: 'test', }); - // Check secrets array exists and not empty - expect(Array.isArray(createdSamlApplication.secrets)).toBe(true); - expect(createdSamlApplication.secrets.length).toBeGreaterThan(0); - - // Check first secret has non-empty privateKey and certificate - // Since we checked the array is not empty in previous check, we can safely access the first element. - const firstSecret = createdSamlApplication.secrets[0]!; - expect(typeof firstSecret.privateKey).toBe('string'); - expect(firstSecret.privateKey).not.toBe(''); - expect(typeof firstSecret.certificate).toBe('string'); - expect(firstSecret.certificate).not.toBe(''); - await deleteSamlApplication(createdSamlApplication.id); }); diff --git a/packages/schemas/src/types/saml-application.ts b/packages/schemas/src/types/saml-application.ts index 08f0ac9c5..10b962ea3 100644 --- a/packages/schemas/src/types/saml-application.ts +++ b/packages/schemas/src/types/saml-application.ts @@ -2,7 +2,6 @@ import { type z } from 'zod'; import { Applications } from '../db-entries/application.js'; import { SamlApplicationConfigs } from '../db-entries/saml-application-config.js'; -import { SamlApplicationSecrets } from '../db-entries/saml-application-secret.js'; import { applicationCreateGuard } from './application.js'; @@ -25,19 +24,10 @@ export const samlApplicationCreateGuard = applicationCreateGuard export type CreateSamlApplication = z.infer; -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. - samlAppConfigGuard - ) - .extend({ - secrets: SamlApplicationSecrets.guard - .omit({ - tenantId: true, - applicationId: true, - }) - .array(), - }); +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. + samlAppConfigGuard +); export type SamlApplicationResponse = z.infer;