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: support SAML app encryption and nameIdFormat config #6912

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
5 changes: 4 additions & 1 deletion packages/core/src/saml-applications/libraries/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,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 @@ -128,7 +128,10 @@
samlConfig,
}: {
application: Application;
samlConfig: Pick<SamlApplicationConfig, 'attributeMapping' | 'entityId' | 'acsUrl'>;
samlConfig: Pick<
SamlApplicationConfig,
'attributeMapping' | 'entityId' | 'acsUrl' | 'encryption' | 'nameIdFormat'
>;

Check warning on line 134 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#L131-L134

Added lines #L131 - L134 were not covered by tests
}): SamlApplicationResponse => {
return {
...application,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ApplicationType, BindingType } from '@logto/schemas';
import { ApplicationType, BindingType, NameIdFormat } from '@logto/schemas';

import { createApplication, deleteApplication, updateApplication } from '#src/api/application.js';
import {
Expand All @@ -24,6 +24,8 @@ describe('SAML application', () => {
description: 'test',
});

expect(createdSamlApplication.nameIdFormat).toBe(NameIdFormat.Persistent);

await deleteSamlApplication(createdSamlApplication.id);
});

Expand Down Expand Up @@ -51,14 +53,25 @@ describe('SAML application', () => {
binding: BindingType.Post,
url: 'https://example.logto.io/sso/saml',
},
nameIdFormat: NameIdFormat.EmailAddress,
encryption: {
encryptAssertion: true,
certificate:
'-----BEGIN CERTIFICATE-----\nMIIDDTCCAfWgAwIBAgI...\n-----END CERTIFICATE-----\n',
encryptThenSign: false,
},
};
const createdSamlApplication = await createSamlApplication({
name: 'test',
description: 'test',
...config,
});

expect(createdSamlApplication.entityId).toEqual(config.entityId);
expect(createdSamlApplication.acsUrl).toEqual(config.acsUrl);
expect(createdSamlApplication.nameIdFormat).toEqual(config.nameIdFormat);
expect(createdSamlApplication.encryption).toEqual(config.encryption);

await deleteSamlApplication(createdSamlApplication.id);
});

Expand All @@ -71,13 +84,19 @@ describe('SAML application', () => {
expect(createdSamlApplication.entityId).toEqual('http://example.logto.io/foo');
expect(createdSamlApplication.acsUrl).toEqual(null);
expect(createdSamlApplication.attributeMapping).toEqual({});
expect(createdSamlApplication.nameIdFormat).toEqual(NameIdFormat.Persistent);
expect(createdSamlApplication.encryption).toBe(null);

const newConfig = {
acsUrl: {
binding: BindingType.Post,
url: 'https://example.logto.io/sso/saml',
},
entityId: null,
nameIdFormat: NameIdFormat.EmailAddress,
encryption: {
encryptAssertion: false,
},
};
const updatedSamlApplication = await updateSamlApplication(createdSamlApplication.id, {
name: 'updated',
Expand All @@ -86,6 +105,8 @@ describe('SAML application', () => {
expect(updatedSamlApplication.acsUrl).toEqual(newConfig.acsUrl);
expect(updatedSamlApplication.entityId).toEqual(newConfig.entityId);
expect(updatedSamlApplication.attributeMapping).toEqual({});
expect(updatedSamlApplication.nameIdFormat).toEqual(newConfig.nameIdFormat);
expect(updatedSamlApplication.encryption).toEqual(newConfig.encryption);

const upToDateSamlApplication = await getSamlApplication(createdSamlApplication.id);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { sql } from '@silverhand/slonik';

import type { AlterationScript } from '../lib/types/alteration.js';

enum NameIdFormat {
/** Uses unique and persistent identifiers for the user. */
Persistent = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent',
}

const alteration: AlterationScript = {
up: async (pool) => {
await pool.query(sql`
alter table saml_application_configs
add column encryption jsonb,
add column name_id_format varchar(128);
`);
await pool.query(sql`
update saml_application_configs
set name_id_format = ${NameIdFormat.Persistent};
`);
await pool.query(sql`
alter table saml_application_configs
alter column name_id_format set not null;
`);
},
down: async (pool) => {
await pool.query(sql`
alter table saml_application_configs
drop column encryption,
drop column name_id_format;
`);
},
};

export default alteration;
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { describe, it, expect } from 'vitest';

import { samlEncryptionGuard } from './saml-application-configs.js';

describe('samlEncryptionGuard', () => {
// Test valid configurations
it('should pass when encryption is disabled', () => {
const result = samlEncryptionGuard.safeParse({
encryptAssertion: false,
});
expect(result.success).toBe(true);
});

it('should pass when encryption is enabled with all required fields', () => {
const result = samlEncryptionGuard.safeParse({
encryptAssertion: true,
encryptThenSign: true,
certificate: '-----BEGIN CERTIFICATE-----\nMIICYDCCAcmgAwIBA...',
});
expect(result.success).toBe(true);
});

// Test invalid configurations
it('should fail when encryptAssertion is true but missing encryptThenSign', () => {
const result = samlEncryptionGuard.safeParse({
encryptAssertion: true,
certificate: '-----BEGIN CERTIFICATE-----\nMIICYDCCAcmgAwIBA...',
});
expect(result.success).toBe(false);
if (!result.success) {
expect(result.error.issues[0]?.message).toBe(
'`encryptThenSign` and `certificate` are required when `encryptAssertion` is `true`'
);
}
});

it('should fail when encryptAssertion is true but missing certificate', () => {
const result = samlEncryptionGuard.safeParse({
encryptAssertion: true,
encryptThenSign: true,
});
expect(result.success).toBe(false);
if (!result.success) {
expect(result.error.issues[0]?.message).toBe(
'`encryptThenSign` and `certificate` are required when `encryptAssertion` is `true`'
);
}
});

it('should fail when encryptAssertion is true but missing both encryptThenSign and certificate', () => {
const result = samlEncryptionGuard.safeParse({
encryptAssertion: true,
});
expect(result.success).toBe(false);
if (!result.success) {
expect(result.error.issues[0]?.message).toBe(
'`encryptThenSign` and `certificate` are required when `encryptAssertion` is `true`'
);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,35 @@ export const samlAcsUrlGuard = z.object({
binding: z.nativeEnum(BindingType),
url: z.string().url(),
}) satisfies ToZodObject<SamlAcsUrl>;

export const samlEncryptionGuard = z
.object({
encryptAssertion: z.boolean().optional(),
encryptThenSign: z.boolean().optional(),
certificate: z.string().optional(),
})
.superRefine(({ encryptAssertion, encryptThenSign, certificate }, ctx) => {
if (encryptAssertion && (encryptThenSign === undefined || certificate === undefined)) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message:
'`encryptThenSign` and `certificate` are required when `encryptAssertion` is `true`',
});
return z.NEVER;
}
});

export type SamlEncryption = z.input<typeof samlEncryptionGuard>;

export enum NameIdFormat {
/** The Identity Provider can determine the format. */
Unspecified = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified',
/** Returns the email address of the user. */
EmailAddress = 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
/** Uses unique and persistent identifiers for the user. */
Persistent = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent',
/** Uses unique and transient identifiers for the user, which can be different for each session. */
Transient = 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
}

export const nameIdFormatGuard = z.nativeEnum(NameIdFormat);
14 changes: 10 additions & 4 deletions packages/schemas/src/types/saml-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import { 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 { nameIdFormatGuard, NameIdFormat } from '../foundations/index.js';

import { applicationCreateGuard, applicationPatchGuard } from './application.js';

const samlAppConfigGuard = SamlApplicationConfigs.guard.pick({
attributeMapping: true,
entityId: true,
acsUrl: true,
encryption: true,
nameIdFormat: true,
});

export const samlApplicationCreateGuard = applicationCreateGuard
Expand All @@ -20,9 +23,10 @@ export const samlApplicationCreateGuard = applicationCreateGuard
customData: true,
})
// The reason for encapsulating attributeMapping and spMetadata into an object within the config field is that you cannot provide only one of `attributeMapping` or `spMetadata`. Due to the structure of the `saml_application_configs` table, both must be not null.
.merge(samlAppConfigGuard.partial());
.merge(samlAppConfigGuard.partial())
.extend({ nameIdFormat: nameIdFormatGuard.optional().default(NameIdFormat.Persistent) });

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

export const samlApplicationPatchGuard = applicationPatchGuard
.pick({
Expand All @@ -31,7 +35,8 @@ export const samlApplicationPatchGuard = applicationPatchGuard
customData: true,
})
// The reason for encapsulating attributeMapping and spMetadata into an object within the config field is that you cannot provide only one of `attributeMapping` or `spMetadata`. Due to the structure of the `saml_application_configs` table, both must be not null.
.merge(samlAppConfigGuard.partial());
.merge(samlAppConfigGuard.partial())
.extend({ nameIdFormat: nameIdFormatGuard.optional() });

export type PatchSamlApplication = z.infer<typeof samlApplicationPatchGuard>;

Expand All @@ -46,7 +51,8 @@ export const samlApplicationResponseGuard = Applications.guard
// 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({ nameIdFormat: nameIdFormatGuard });

export type SamlApplicationResponse = z.infer<typeof samlApplicationResponseGuard>;

Expand Down
2 changes: 2 additions & 0 deletions packages/schemas/tables/saml_application_configs.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ create table saml_application_configs (
attribute_mapping jsonb /* @use SamlAttributeMapping */ not null default '{}'::jsonb,
entity_id varchar(128),
acs_url jsonb /* @use SamlAcsUrl */,
encryption jsonb /* @use SamlEncryption */,
name_id_format varchar(128) /* @use NameIdFormat */ not null,
primary key (tenant_id, application_id),
constraint saml_application_configs__application_type
check (check_application_type(application_id, 'SAML'))
Expand Down
Loading