Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add SAML app anonymous metadata and certificate APIs #6833

Merged
merged 4 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/core/src/routes/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import koaBodyEtag from '#src/middleware/koa-body-etag.js';
import { koaManagementApiHooks } from '#src/middleware/koa-management-api-hooks.js';
import koaTenantGuard from '#src/middleware/koa-tenant-guard.js';
import samlApplicationAnonymousRoutes from '#src/saml-applications/routes/anonymous.js';
import samlApplicationRoutes from '#src/saml-applications/routes/index.js';
import type TenantContext from '#src/tenants/TenantContext.js';

Expand Down Expand Up @@ -68,7 +69,7 @@
managementRouter.use(koaTenantGuard(tenant.id, tenant.queries));
managementRouter.use(koaManagementApiHooks(tenant.libraries.hooks));

// TODO: FIXME @sijie @darcy mount these routes in `applicationRoutes` instead

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: FIXME @sijie @darcy mount these...'.
applicationRoutes(managementRouter, tenant);
applicationRoleRoutes(managementRouter, tenant);
applicationProtectedAppMetadataRoutes(managementRouter, tenant);
Expand Down Expand Up @@ -100,7 +101,7 @@
systemRoutes(managementRouter, tenant);
subjectTokenRoutes(managementRouter, tenant);
accountCentersRoutes(managementRouter, tenant);
// 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 104 in packages/core/src/routes/init.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: @darcy per our design, we will...'.
if (
(EnvSet.values.isDevFeaturesEnabled && EnvSet.values.isCloud) ||
EnvSet.values.isIntegrationTest
Expand All @@ -112,7 +113,7 @@

const userRouter: UserRouter = new Router();
userRouter.use(koaOidcAuth(tenant));
// TODO(LOG-10147): Rename to koaApiHooks, this middleware is used for both management API and user API

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO(LOG-10147): Rename to koaApiHooks,...'.
userRouter.use(koaManagementApiHooks(tenant.libraries.hooks));
accountRoutes(userRouter, tenant);
verificationRoutes(userRouter, tenant);
Expand All @@ -120,6 +121,13 @@
wellKnownRoutes(anonymousRouter, tenant);
statusRoutes(anonymousRouter, tenant);
authnRoutes(anonymousRouter, tenant);
// 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 124 in packages/core/src/routes/init.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

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

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

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/init.ts#L129-L130

Added lines #L129 - L130 were not covered by tests

wellKnownOpenApiRoutes(anonymousRouter, {
experienceRouters: [experienceRouter, interactionRouter],
Expand All @@ -133,7 +141,7 @@
anonymousRouter,
experienceRouter,
userRouter,
// TODO: interactionRouter should be removed from swagger.json

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: interactionRouter should be...'.
interactionRouter,
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,30 @@
type SamlApplicationResponse,
type PatchSamlApplication,
type SamlApplicationSecret,
BindingType,
} from '@logto/schemas';
import { generateStandardId } from '@logto/shared';
import { removeUndefinedKeys } from '@silverhand/essentials';
import * as saml from 'samlify';

import { EnvSet, getTenantEndpoint } from '#src/env-set/index.js';
import RequestError from '#src/errors/RequestError/index.js';
import type Queries from '#src/tenants/Queries.js';
import assertThat from '#src/utils/assert-that.js';

import { ensembleSamlApplication, generateKeyPairAndCertificate } from './utils.js';
import {
ensembleSamlApplication,
generateKeyPairAndCertificate,
buildSingleSignOnUrl,
} from './utils.js';

export const createSamlApplicationsLibrary = (queries: Queries) => {
const {
applications: { findApplicationById, updateApplicationById },
samlApplicationSecrets: { insertSamlApplicationSecret },
samlApplicationSecrets: {
insertSamlApplicationSecret,
findActiveSamlApplicationSecretByApplicationId,
},
samlApplicationConfigs: {
findSamlApplicationConfigByApplicationId,
updateSamlApplicationConfig,
Expand All @@ -25,8 +35,8 @@

const createSamlApplicationSecret = async (
applicationId: string,
// Set certificate life span to 1 year by default.
lifeSpanInDays = 365
// Set certificate life span to 3 years by default.
lifeSpanInDays = 365 * 3

Check warning on line 39 in packages/core/src/saml-applications/libraries/saml-applications.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/libraries/saml-applications.ts#L38-L39

Added lines #L38 - L39 were not covered by tests
): Promise<SamlApplicationSecret> => {
const { privateKey, certificate, notAfter } = await generateKeyPairAndCertificate(
lifeSpanInDays
Expand All @@ -37,7 +47,7 @@
applicationId,
privateKey,
certificate,
expiresAt: Math.floor(notAfter.getTime() / 1000),
expiresAt: notAfter.getTime(),

Check warning on line 50 in packages/core/src/saml-applications/libraries/saml-applications.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/libraries/saml-applications.ts#L50

Added line #L50 was not covered by tests
active: false,
});
};
Expand Down Expand Up @@ -91,9 +101,37 @@
});
};

const getSamlIdPMetadataByApplicationId = async (id: string): Promise<{ metadata: string }> => {
const [{ tenantId, entityId }, { certificate }] = await Promise.all([
findSamlApplicationConfigByApplicationId(id),
findActiveSamlApplicationSecretByApplicationId(id),
]);

assertThat(entityId, 'application.saml.entity_id_required');

const tenantEndpoint = getTenantEndpoint(tenantId, EnvSet.values);

// eslint-disable-next-line new-cap
const idp = saml.IdentityProvider({
entityID: entityId,
signingCert: certificate,
singleSignOnService: [
{
Location: buildSingleSignOnUrl(tenantEndpoint, id),
Binding: BindingType.Redirect,
},
],
});

return {
metadata: idp.getMetadata(),
};

Check warning on line 128 in packages/core/src/saml-applications/libraries/saml-applications.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/libraries/saml-applications.ts#L105-L128

Added lines #L105 - L128 were not covered by tests
};

return {
createSamlApplicationSecret,
findSamlApplicationById,
updateSamlApplicationById,
getSamlIdPMetadataByApplicationId,
};
};
70 changes: 69 additions & 1 deletion packages/core/src/saml-applications/libraries/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { addDays } from 'date-fns';
import forge from 'node-forge';

import { generateKeyPairAndCertificate } from './utils.js';
import RequestError from '#src/errors/RequestError/index.js';

import { generateKeyPairAndCertificate, calculateCertificateFingerprints } from './utils.js';

describe('generateKeyPairAndCertificate', () => {
it('should generate valid key pair and certificate', async () => {
Expand Down Expand Up @@ -57,3 +59,69 @@ describe('generateKeyPairAndCertificate', () => {
expect(forge.pki.privateKeyToPem(privateKey).length).toBeGreaterThan(3000); // A 4096-bit RSA private key in PEM format is typically longer than 3000 characters
});
});

describe('calculateCertificateFingerprints', () => {
// eslint-disable-next-line @silverhand/fp/no-let
let validCertificate: string;

beforeAll(async () => {
// Generate a valid certificate for testing
const { certificate } = await generateKeyPairAndCertificate();
// eslint-disable-next-line @silverhand/fp/no-mutation
validCertificate = certificate;
});

it('should calculate correct fingerprints for valid certificate', () => {
const fingerprints = calculateCertificateFingerprints(validCertificate);

// Verify fingerprint format
expect(fingerprints.sha256.formatted).toMatch(/^([\dA-F]{2}:){31}[\dA-F]{2}$/);
expect(fingerprints.sha256.unformatted).toMatch(/^[\dA-F]{64}$/);

// Verify formatted and unformatted consistency
expect(fingerprints.sha256.unformatted).toBe(fingerprints.sha256.formatted.replaceAll(':', ''));
});

it('should throw error for invalid PEM format', () => {
const invalidCertificates = [
'not a certificate',
'-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n',
// Missing begin/end markers
'MIIFWjCCA0KgAwIBAgIUMDAwMDAwMDAwMDAwMDAwMDAwMDAwDQYJKoZIhvcNAQEL',
];

for (const cert of invalidCertificates) {
expect(() => calculateCertificateFingerprints(cert)).toThrow(
new RequestError('application.saml.invalid_certificate_pem_format')
);
}
});

it('should throw error for invalid base64 content', () => {
const invalidBase64Certificate =
'-----BEGIN CERTIFICATE-----\n' +
'This is not base64!@#$%^&*()\n' +
'-----END CERTIFICATE-----\n';

expect(() => calculateCertificateFingerprints(invalidBase64Certificate)).toThrow(
new RequestError('application.saml.invalid_certificate_pem_format')
);
});

it('should handle certificates with different line endings', () => {
// Replace \n with \r\n in valid certificate
const crlfCertificate = validCertificate.replaceAll('\n', '\r\n');

const originalFingerprints = calculateCertificateFingerprints(validCertificate);
const crlfFingerprints = calculateCertificateFingerprints(crlfCertificate);

expect(crlfFingerprints).toEqual(originalFingerprints);
});

it('should calculate consistent fingerprints for the same certificate', () => {
const firstResult = calculateCertificateFingerprints(validCertificate);
const secondResult = calculateCertificateFingerprints(validCertificate);

expect(firstResult).toEqual(secondResult);
});
});
55 changes: 54 additions & 1 deletion packages/core/src/saml-applications/libraries/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,25 @@
type SamlApplicationConfig,
type SamlAcsUrl,
BindingType,
type CertificateFingerprints,
} from '@logto/schemas';
import { appendPath } from '@silverhand/essentials';
import { addDays } from 'date-fns';
import forge from 'node-forge';
import { z } from 'zod';

import RequestError from '#src/errors/RequestError/index.js';
import assertThat from '#src/utils/assert-that.js';

// Add PEM certificate validation
const pemCertificateGuard = z
.string()
.trim()
.regex(/^-{5}BEGIN CERTIFICATE-{5}[\n\r]+[\S\s]*?[\n\r]+-{5}END CERTIFICATE-{5}$/);

// Add base64 validation schema
const base64Guard = z.string().regex(/^[\d+/A-Za-z]*={0,2}$/);

export const generateKeyPairAndCertificate = async (lifeSpanInDays = 365) => {
const keypair = forge.pki.rsa.generateKeyPair({ bits: 4096 });
return createCertificate(keypair, lifeSpanInDays);
Expand All @@ -33,7 +45,7 @@
cert.validity.notAfter = notAfter;
/* eslint-enable @silverhand/fp/no-mutation */

// TODO: read from tenant config or let user customize before downloading

Check warning on line 48 in packages/core/src/saml-applications/libraries/utils.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/saml-applications/libraries/utils.ts#L48

[no-warning-comments] Unexpected 'todo' comment: 'TODO: read from tenant config or let...'.
const subjectAttributes: forge.pki.CertificateField[] = [
{
name: 'commonName',
Expand Down Expand Up @@ -67,6 +79,44 @@
};
};

export const calculateCertificateFingerprints = (
pemCertificate: string
): CertificateFingerprints => {
try {
// Validate PEM certificate format
pemCertificateGuard.parse(pemCertificate);

// Remove PEM headers, newlines and spaces
const cleanedPem = pemCertificate
.replace('-----BEGIN CERTIFICATE-----', '')
.replace('-----END CERTIFICATE-----', '')
.replaceAll(/\s/g, '');

// Validate base64 format using zod
base64Guard.parse(cleanedPem);

// Convert base64 to binary
const certDer = Buffer.from(cleanedPem, 'base64');

// Calculate SHA-256 fingerprint
const sha256Unformatted = crypto
.createHash('sha256')
.update(certDer)
.digest('hex')
.toUpperCase();
const sha256Formatted = sha256Unformatted.match(/.{2}/g)?.join(':') ?? '';

return {
sha256: {
formatted: sha256Formatted,
unformatted: sha256Unformatted,
},
};
} catch {
throw new RequestError('application.saml.invalid_certificate_pem_format');
}
};

/**
* 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:
Expand All @@ -92,10 +142,13 @@
export const validateAcsUrl = (acsUrl: SamlAcsUrl) => {
const { binding } = acsUrl;
assertThat(
binding === BindingType.POST,
binding === BindingType.Post,

Check warning on line 145 in packages/core/src/saml-applications/libraries/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/libraries/utils.ts#L145

Added line #L145 was not covered by tests
new RequestError({
code: 'application.saml.acs_url_binding_not_supported',
status: 422,
})
);
};

export const buildSingleSignOnUrl = (baseUrl: URL, samlApplicationId: string) =>
appendPath(baseUrl, `api/saml/${samlApplicationId}/authn`).toString();
5 changes: 0 additions & 5 deletions packages/core/src/saml-applications/queries/configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ export const createSamlApplicationConfigQueries = (pool: CommonQueryMethods) =>
const findSamlApplicationConfigByApplicationId = async (applicationId: string) =>
/**
* @remarks
* 这里我们使用了 `.one()` 方法 instead of `.maybeOne()` 方法,是因为我们在创建 SAML app 时,直接会创建一条对应的 SAML config 记录。这样在后续我们通过 API 操作 SAML app 的 config 时,不需要额外判断:
* 1. 是否需要插入 SAML config(一个 alternative 是,在创建 SAML app 时,不插入一条 SAML config 记录,在 PATCH 时,使用 insert into ... on conflict 的 query 来达到同样的目的)
* 2. 在 SAML app 对应的 config 不存在时,GET 方法需要对 null 的 SAML config 进行额外的处理
* 根据我们的设计,如果在创建 SAML app 时,就同时创建一条 SAML config 记录,在之后的所有场景中,我们只涉及对这条 DB 记录的 update 操作。在我们的业务场景中,没有手动对 SAML config 记录的 delete 操作。
*
* Here we use the `.one()` method instead of the `.maybeOne()` method because when creating a SAML app, we directly create a corresponding SAML config record. This means that in subsequent API operations on the SAML app's config, we don't need additional checks:
* 1. Whether to insert a SAML config (an alternative approach is not to insert a SAML config record when creating the SAML app, and use an `insert into ... on conflict` query during PATCH to achieve the same result).
* 2. When the corresponding config for the SAML app does not exist, the GET method needs to handle the null SAML config additionally.
Expand Down
18 changes: 18 additions & 0 deletions packages/core/src/saml-applications/queries/secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { sql } from '@silverhand/slonik';

import { buildInsertIntoWithPool } from '#src/database/insert-into.js';
import RequestError from '#src/errors/RequestError/index.js';
import { DeletionError } from '#src/errors/SlonikError/index.js';
import { convertToIdentifiers } from '#src/utils/sql.js';

Expand All @@ -20,6 +21,22 @@
where ${fields.applicationId}=${applicationId}
`);

const findActiveSamlApplicationSecretByApplicationId = async (
applicationId: string
): Promise<SamlApplicationSecret> => {
const activeSecret = await pool.maybeOne<SamlApplicationSecret>(sql`
select ${sql.join(Object.values(fields), sql`, `)}
from ${table}
where ${fields.applicationId}=${applicationId} and ${fields.active}=true
`);

if (!activeSecret) {
throw new RequestError({ code: 'application.saml.no_active_secret', status: 404 });
}

return activeSecret;

Check warning on line 37 in packages/core/src/saml-applications/queries/secrets.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/queries/secrets.ts#L25-L37

Added lines #L25 - L37 were not covered by tests
};

const findSamlApplicationSecretByApplicationIdAndId = async (applicationId: string, id: string) =>
pool.one<SamlApplicationSecret>(sql`
select ${sql.join(Object.values(fields), sql`, `)}
Expand Down Expand Up @@ -68,6 +85,7 @@
return {
insertSamlApplicationSecret,
findSamlApplicationSecretsByApplicationId,
findActiveSamlApplicationSecretByApplicationId,
findSamlApplicationSecretByApplicationIdAndId,
deleteSamlApplicationSecretById,
updateSamlApplicationSecretStatusByApplicationIdAndSecretId,
Expand Down
32 changes: 32 additions & 0 deletions packages/core/src/saml-applications/routes/anonymous.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { z } from 'zod';

import koaGuard from '#src/middleware/koa-guard.js';
import type { AnonymousRouter, RouterInitArgs } from '#src/routes/types.js';

export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter>(
...[router, { libraries }]: RouterInitArgs<T>
) {
const {
samlApplications: { getSamlIdPMetadataByApplicationId },
} = libraries;

router.get(
'/saml-applications/:id/metadata',
koaGuard({
params: z.object({ id: z.string() }),
status: [200, 400, 404],
response: z.string(),
}),
async (ctx, next) => {
const { id } = ctx.guard.params;

const { metadata } = await getSamlIdPMetadataByApplicationId(id);

ctx.status = 200;
ctx.body = metadata;
ctx.type = 'text/xml;charset=utf-8';

return next();
}
);
}

Check warning on line 32 in packages/core/src/saml-applications/routes/anonymous.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/routes/anonymous.ts#L7-L32

Added lines #L7 - L32 were not covered by tests
Loading
Loading