Skip to content

Commit

Permalink
chore: update code
Browse files Browse the repository at this point in the history
  • Loading branch information
darcyYe committed Nov 27, 2024
1 parent ccb47b7 commit 3d85ab1
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 156 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 5 additions & 1 deletion packages/core/src/routes/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Check warning on line 103 in packages/core/src/routes/init.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/routes/init.ts#L103

[no-warning-comments] Unexpected 'todo' comment: 'TODO: @darcy per our design, we will...'.
if (
(EnvSet.values.isDevFeaturesEnabled && EnvSet.values.isCloud) ||
EnvSet.values.isIntegrationTest
) {
samlApplicationRoutes(managementRouter, tenant);
}

Check warning on line 109 in packages/core/src/routes/init.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/init.ts#L108-L109

Added lines #L108 - L109 were not covered by tests

Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/routes/swagger/utils/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ export const buildRouterObjects = <T extends UnknownRouter>(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.

Check warning on line 136 in packages/core/src/routes/swagger/utils/operation.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/routes/swagger/utils/operation.ts#L136

[no-warning-comments] Unexpected 'todo' comment: 'TODO: Remove this and bring back...'.
// Exclude `/saml-applications` routes for now.
.filter(({ path }) => !path.startsWith('/saml-applications'))
.flatMap<RouteObject>(({ 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);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/saml-applications/libraries/secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,6 +29,6 @@ export const createSamlApplicationSecretsLibrary = (queries: Queries) => {
};

return {
createNewSamlApplicationSecretForApplication,
createSamlApplicationSecret,
};
};
59 changes: 59 additions & 0 deletions packages/core/src/saml-applications/libraries/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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
});
});
95 changes: 0 additions & 95 deletions packages/core/src/saml-applications/routes/index.openapi.json

This file was deleted.

44 changes: 20 additions & 24 deletions packages/core/src/saml-applications/routes/index.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
import {
ApplicationType,
BindingType,
samlApplicationCreateGuard,
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 } 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<T extends ManagementApiRouter>(
Expand All @@ -24,7 +22,7 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
samlApplicationConfigs: { insertSamlApplicationConfig },
} = queries;
const {
samlApplicationSecrets: { createNewSamlApplicationSecretForApplication },
samlApplicationSecrets: { createSamlApplicationSecret },
} = libraries;

router.post(
Expand All @@ -37,12 +35,8 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
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(
Expand All @@ -58,16 +52,21 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
})
);

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();
}
Expand All @@ -82,11 +81,8 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
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);

Expand Down
28 changes: 24 additions & 4 deletions packages/core/src/saml-applications/routes/utils.ts
Original file line number Diff line number Diff line change
@@ -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<SamlApplicationConfig, 'attributeMapping' | 'entityId' | 'acsUrl'>;
samlSecret?: SamlApplicationSecret | SamlApplicationSecret[];
}): SamlApplicationResponse => {
return {
...application,
...samlConfig,
secrets: samlSecret ? (Array.isArray(samlSecret) ? samlSecret : [samlSecret]) : [],
};
};

Check warning on line 29 in packages/core/src/saml-applications/routes/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/routes/utils.ts#L19-L29

Added lines #L19 - L29 were not covered by tests

/**
* 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,
})
);
};

Check warning on line 42 in packages/core/src/saml-applications/routes/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/routes/utils.ts#L35-L42

Added lines #L35 - L42 were not covered by tests
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
20 changes: 5 additions & 15 deletions packages/schemas/src/types/saml-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -25,19 +24,10 @@ export const samlApplicationCreateGuard = applicationCreateGuard

export type CreateSamlApplication = z.infer<typeof samlApplicationCreateGuard>;

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<typeof samlApplicationResponseGuard>;

0 comments on commit 3d85ab1

Please sign in to comment.