diff --git a/README.md b/README.md index 4bd462b9..c336094e 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,8 @@ -SAML 2.0 & 1.1 Assertion Parser & Validator +# SAML 2.0 parser and validator -![Build Status](https://github.com/boxyhq/saml20/actions/workflows/node.js.yml/badge.svg) +![Build Status](https://github.com/ory/saml20/actions/workflows/node.js.yml/badge.svg) -============= - -boxyhq/saml20 is a fork of a fork of [saml20](https://github.com/leandrob/saml20). It now has extended functionality and diverges from the original unmaintained library. The new package is published here - https://www.npmjs.com/package/@boxyhq/saml20 +A SAML 2.0 library for Node.js: it validates SAML Responses, and creates and parses AuthnRequests, logout messages and metadata. It started as a fork of [saml20](https://github.com/leandrob/saml20) and has since diverged from it. Ory maintains it, and it is published on npm as [`@boxyhq/saml20`](https://www.npmjs.com/package/@boxyhq/saml20). ## Installation @@ -14,7 +12,7 @@ $ npm install @boxyhq/saml20 ## Usage -`parse()` and `validate()` return promises. Before 1.6.0 they took a callback as their last argument; since 1.6.0 a callback is ignored and is never called, so `await` the result or use `.then()`/`.catch()` instead. +`validate()` returns a promise. ### saml.validate(rawResponse, options) @@ -27,8 +25,7 @@ Verifies a SAML Response (or a lone Assertion) and resolves with the user's prof - `publicKey`: the trusted IdP signing certificate, as base64 DER or PEM. Several certificates (for key rotation) are separated by commas. - `thumbprint`: the SHA-1 thumbprint(s) of a trusted certificate, comma-separated. The certificate itself is taken from the `KeyInfo` of the signature being verified. Pass either `publicKey` or `thumbprint`, not both. - `privateKey` (optional): decrypts an `EncryptedAssertion`. -- `audience` (optional): the expected audience, or a list of accepted ones. When set, every `AudienceRestriction` in the assertion must name one of them. -- `strictAudienceValidation` (optional): require an exact audience match. Without it, an audience that merely starts with the expected value is accepted. +- `audience`: the expected audience, or a list of accepted ones. Every `AudienceRestriction` in the assertion must name one of them exactly. Fails with `Invalid audience.` A missing or empty value fails with `audience is required.` - `inResponseTo` (optional): the ID of the `AuthnRequest` this response answers. When set, the signed content must carry the same `InResponseTo`; a missing value fails. Fails with `Invalid InResponseTo.` - `recipient` (optional). The ACS URL the response was posted to. When set, the signed content must name it: a signed `Response/@Destination` must equal it, when any bearer `SubjectConfirmationData` carries a `Recipient`, one bearer confirmation must both name it and be within its own `NotBefore`/`NotOnOrAfter` window (so `NotOnOrAfter` is required on it; `bypassExpiration` skips this window check but not the `Recipient` match), and at least one of the two must be present. An empty `Recipient` counts as present and never matches. The unsigned `Response` wrapper is never used. Fails with `Invalid Recipient.` - `assertionReplayValidator` (optional): `(info) => boolean | Promise`, called after every other check passes, with `{ assertionId, sessionIndex, notOnOrAfter, inResponseTo }` from the signed assertion. Return `true` if the assertion was already used, and the response is rejected as a replay. Keep each entry until at least `notOnOrAfter` plus 10 minutes, the clock skew `validate()` allows. @@ -42,7 +39,6 @@ const saml = require('@boxyhq/saml20').default; const profile = await saml.validate(rawResponse, { publicKey: 'MIICDzCCAXygAwIBAgIQVWXAvbbQyI5Bc...', audience: 'https://sp.example.com', - strictAudienceValidation: true, inResponseTo: requestId, recipient: 'https://sp.example.com/acs', }); @@ -60,13 +56,9 @@ Returns the `Issuer` of a SAML Response without validating anything. Use it to f const issuer = saml.parseIssuer(rawResponse); ``` -### [DEPRECATED] saml.parse(rawResponse) - -Resolves with the same profile as `validate()`, but checks nothing: not the signature, expiry, audience or anything else. Anyone can forge its output, so never use it to authenticate a user. Use `validate()` for that, and `parseIssuer()` to read the Issuer before validating. - ### saml.validateSignature(xml, publicKey, thumbprint, options?) -Verifies the XML signature on `xml` and returns the signed XML, or throws / returns `null` when the signature is invalid. Pass either `publicKey` (one certificate, or several separated by commas for key rotation) or `thumbprint`, not both. +Verifies the XML signature on `xml` and returns the signed XML. Read the document from that return value, not from `xml`: only the signed content is covered. It throws when the signature is invalid or cannot be checked; a signature that does not verify fails with `invalid signature: the signature does not verify`. Pass either `publicKey` (one certificate, or several separated by commas for key rotation) or `thumbprint`, not both. `options` (optional) restricts which algorithms are accepted: @@ -92,6 +84,19 @@ var signedXml = saml.validateSignature(xml, publicKey, null, { }); ``` +## Upgrading to 2.0 + +2.0 removes APIs that were deprecated or that Polis no longer uses: + +- Only the package root can be imported. `package.json` now has an `exports` map, so paths inside the package such as `@boxyhq/saml20/dist/typings` fail with `ERR_PACKAGE_PATH_NOT_EXPORTED` (and with TS2307 under `moduleResolution` `node16`, `nodenext` or `bundler`). Import from the root instead, for example `import type { SAMLProfile } from '@boxyhq/saml20'`. Everything the root exports is listed in `test/lib/compat.spec.ts`. +- `parse()` is gone. It returned a profile without checking the signature, expiry, audience or anything else, so its output could be forged. Use `validate()` to authenticate a response, and `parseIssuer()` to read the Issuer before validating (for example, to pick the IdP certificate). +- `decryptXml` and `PubKeyInfo` are no longer exported from the package root. They were internal helpers for `validate()` and `sign()`. +- `hasValidSignature` is removed. Use `validateSignature()`, which also rejects being given both a certificate and a thumbprint. +- `isMultiRootedXMLError` is removed. A multi-rooted document is rejected with the error `multirooted xml not allowed.` +- `strictAudienceValidation` is gone and audiences are always compared exactly. Before 2.0, without that option an `Audience` that merely started with the expected value was accepted (`https://sp.example.com/metadata.evil.example` passed for `https://sp.example.com/metadata`). If you relied on a prefix match, pass every accepted audience in `audience` instead. +- `validateSignature()` throws when the signature does not verify, where it used to return `null`. It now either returns the signed XML or throws. +- `audience` is required. Before 2.0, leaving it out skipped the audience check, so an assertion issued for any other service provider that trusts the same IdP was accepted. + ## Tests ```bash @@ -106,18 +111,13 @@ Fixtures live in `test/assets`. Tests that need a fresh validity window, or a sh Thanks for taking the time to contribute! Contributions are what make the open-source community such an amazing place to learn, inspire, and create. Any contributions you make will benefit everybody and are appreciated. -Please try to create bug reports that are: +Report bugs in [GitHub issues](https://github.com/ory/saml20/issues). Please try to create bug reports that are: - _Reproducible._ Include steps to reproduce the problem. - _Specific._ Include as much detail as possible: which version, what environment, etc. - _Unique._ Do not duplicate existing opened issues. - _Scoped to a Single Bug._ One bug per report. -## Community - -- [Discord](https://discord.gg/uyb7pYt4Pa) (For live discussion with the Community and BoxyHQ team) -- [Twitter](https://twitter.com/BoxyHQ) (Get the news fast) - ## Reporting Security Issues [Responsible Disclosure](SECURITY.md) diff --git a/lib/index.ts b/lib/index.ts index e6b087ed..1d1eff08 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -1,4 +1,4 @@ -import { validateSignature, certToPEM, DEFAULT_MAX_SIGNATURE_REFERENCES } from './validateSignature'; +import { validateSignature, DEFAULT_MAX_SIGNATURE_REFERENCES } from './validateSignature'; import type { ValidateSignatureOptions } from './validateSignature'; import { @@ -11,12 +11,11 @@ import { MAX_INFLATED_REQUEST_BYTES, } from './request'; import type { DecodeBase64Options } from './request'; -import { stripCertHeaderAndFooter, PubKeyInfo } from './cert'; -import { createSAMLResponse, parse, validate, parseIssuer, WrapError } from './response'; +import { stripCertHeaderAndFooter, certToPEM } from './cert'; +import { createSAMLResponse, validate, parseIssuer, WrapError } from './response'; import { parseMetadata, createIdPMetadataXML, createSPMetadataXML } from './metadata'; import { createPostForm } from './post'; import { sign } from './sign'; -import { decryptXml } from './decrypt'; import { containsDoctype } from './utils'; import { parseLogoutResponse, createLogoutRequest, parseLogoutRequest, createLogoutResponse } from './logout'; import type { @@ -38,16 +37,13 @@ export default { samlRequestDecodeError, MAX_ENCODED_REQUEST_LENGTH, MAX_INFLATED_REQUEST_BYTES, - parse, validate, - PubKeyInfo, certToPEM, stripCertHeaderAndFooter, createPostForm, sign, validateSignature, DEFAULT_MAX_SIGNATURE_REFERENCES, - decryptXml, parseIssuer, WrapError, containsDoctype, @@ -71,16 +67,13 @@ export { samlRequestDecodeError, MAX_ENCODED_REQUEST_LENGTH, MAX_INFLATED_REQUEST_BYTES, - parse, validate, - PubKeyInfo, certToPEM, stripCertHeaderAndFooter, createPostForm, sign, validateSignature, DEFAULT_MAX_SIGNATURE_REFERENCES, - decryptXml, parseIssuer, WrapError, containsDoctype, diff --git a/lib/response.ts b/lib/response.ts index e823ca06..ecd919a4 100644 --- a/lib/response.ts +++ b/lib/response.ts @@ -2,7 +2,7 @@ import type { SAMLProfile, ValidateOptions } from './typings'; import xml2js from 'xml2js'; import xmlbuilder from 'xmlbuilder'; import { getVersion } from './getVersion'; -import { validateSignature, sanitizeXML } from './validateSignature'; +import { verifySignature, sanitizeXML } from './validateSignature'; import type { ValidateSignatureOptions } from './validateSignature'; import { decryptXml } from './decrypt'; import { select } from 'xpath'; @@ -26,31 +26,6 @@ const nameFormatUri = [ 'urn:mace:dir:attribute-def:jpegPhoto', ]; -/** - * Parses a SAML Response or Assertion into a profile WITHOUT checking its - * signature, expiry, audience or anything else. Its output can be forged by - * whoever sends the document. - * - * @deprecated Use validate() to authenticate a response. To read the Issuer - * before validating (for example, to choose which IdP certificate to pass to - * validate()), use parseIssuer(). - */ -const parse = async (rawAssertion: string): Promise => { - if (!rawAssertion) { - throw new Error('rawAssertion is required.'); - } - - const assertionObj = await xmlToJs(rawAssertion); - - const statusError = getStatusCodeError(assertionObj); - if (statusError) { - throw statusError; - } - - const { assertion } = parseResponseAndVersion(assertionObj); - return parseAttributes(assertion); -}; - const parseIssuer = (rawAssertion) => { if (!rawAssertion) { throw new Error('rawAssertion is required.'); @@ -82,6 +57,14 @@ const validate = async (rawAssertion: string, options: ValidateOptions): Promise throw new Error('You should provide either cert or certThumbprint, not both'); } + const audiences: unknown[] = Array.isArray(options.audience) ? options.audience : [options.audience]; + if ( + audiences.length === 0 || + !audiences.every((audience) => typeof audience === 'string' && audience !== '') + ) { + throw new Error('audience is required.'); + } + // Optional algorithm allowlists and the Reference ceiling, forwarded to // validateSignature. Leaving them out keeps the defaults. const signatureOptions: ValidateSignatureOptions = { @@ -153,7 +136,7 @@ const checkSignature = ( signatureOptions: ValidateSignatureOptions ): string | null => { try { - return validateSignature(xml, options.publicKey, options.thumbprint, signatureOptions); + return verifySignature(xml, options.publicKey, options.thumbprint, signatureOptions); } catch (e) { const error = new WrapError('Invalid assertion.'); error.inner = e; @@ -175,10 +158,7 @@ const checkAssertion = async ( return new Error('Assertion is expired.'); } - if ( - options.audience && - !saml20.validateAudience(assertion, options.audience, options.strictAudienceValidation) - ) { + if (!saml20.validateAudience(assertion, options.audience)) { return new Error('Invalid audience.'); } @@ -269,9 +249,10 @@ const getStatusCodeError = (responseObj): Error | undefined => { }; /* - assertionObj: the signed content, either a whole SAML Response or a lone - Assertion. Only signed content is read here, never the unsigned - wrapper around a signed Assertion. + assertionObj: the signed content: a whole SAML Response, a lone Assertion, + or a whole WS-Trust RequestSecurityTokenResponse (ADFS over WS-Federation). + Only signed content is read here, never an unsigned wrapper around a signed + Assertion. */ function parseResponseAndVersion(assertionObj) { let assertion = @@ -496,4 +477,4 @@ const createSAMLResponse = async ({ return signedXml; }; -export { createSAMLResponse, parse, validate, parseIssuer, WrapError }; +export { createSAMLResponse, validate, parseIssuer, WrapError }; diff --git a/lib/saml20.ts b/lib/saml20.ts index 26ac9f07..5b3c8ddf 100644 --- a/lib/saml20.ts +++ b/lib/saml20.ts @@ -140,17 +140,10 @@ const parse = (assertion) => { }; }; -const audienceCheck = (audience, expectedAudience, strictValidation) => { - if (typeof audience !== 'string' || typeof expectedAudience !== 'string') { - return false; - } - - if (strictValidation) { - return audience === expectedAudience; - } - - return audience.startsWith(expectedAudience); -}; +// Audiences are compared exactly. A prefix match would accept an assertion +// issued for any identifier that merely starts with the relying party's. +const audienceCheck = (audience, expectedAudience) => + typeof audience === 'string' && typeof expectedAudience === 'string' && audience === expectedAudience; const toArray = (value: T | T[] | undefined | null): T[] => { if (value === undefined || value === null) { @@ -169,8 +162,9 @@ const getAudienceRestrictions = (assertion): unknown[][] => // SAML core 2.5.1.4: an assertion may carry several // elements, and each must be satisfied; within one the // values are alternatives, so any one of them may name the relying -// party. `realm` is the relying party's own identifier, or a list of them. -const validateAudience = (assertion, realm, strictValidation = false) => { +// party. `realm` is the relying party's own identifier, or a list of them; +// an Audience must equal one of them exactly. +const validateAudience = (assertion, realm) => { try { const expected = toArray(realm); const restrictions = getAudienceRestrictions(assertion); @@ -180,7 +174,7 @@ const validateAudience = (assertion, realm, strictValidation = false) => { return restrictions.every((audiences) => { return audiences.some((audience) => - expected.some((expectedAudience) => audienceCheck(audience, expectedAudience, strictValidation)) + expected.some((expectedAudience) => audienceCheck(audience, expectedAudience)) ); }); // eslint-disable-next-line @typescript-eslint/no-unused-vars diff --git a/lib/typings.ts b/lib/typings.ts index 1a091f09..19d7383e 100644 --- a/lib/typings.ts +++ b/lib/typings.ts @@ -65,10 +65,9 @@ export interface ValidateOptions extends ValidateSignatureOptions { thumbprint?: string; // Private key that decrypts an EncryptedAssertion. privateKey?: string; - // Expected audience, or several accepted audiences. - audience?: string | string[]; - // Require an exact audience match instead of a prefix match. - strictAudienceValidation?: boolean; + // Expected audience, or several accepted audiences. Every + // AudienceRestriction must name one of them exactly. + audience: string | string[]; // ID of the AuthnRequest this response answers (SP-initiated flows). When // set, the signed content must carry the same InResponseTo. inResponseTo?: string; diff --git a/lib/utils.ts b/lib/utils.ts index e9f53a73..fdc8503b 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -136,7 +136,6 @@ const getAttribute = (value: any, path: string, defaultValue return current; }; -/** @deprecated An internal helper; it stops being exported in the next major version. */ const isMultiRootedXMLError = (err: any) => { if ((err as any)?.message?.indexOf('Only one element can be added and only after doctype') >= 0) { return true; @@ -150,7 +149,6 @@ export { parseFromString, thumbprint, getAttribute, - isMultiRootedXMLError, multiRootedXMLError, doctypeNotAllowedError, containsDoctype, diff --git a/lib/validateSignature.ts b/lib/validateSignature.ts index 6e97735b..badb2bf9 100644 --- a/lib/validateSignature.ts +++ b/lib/validateSignature.ts @@ -159,16 +159,7 @@ const assertBoundedReferences = (signature, options?: ValidateSignatureOptions) } }; -/** - * @deprecated Use validateSignature, which also rejects being given both a - * certificate and a thumbprint. It stops being exported in the next major version. - */ const hasValidSignature = (xml, cert, certThumbprint, options?: ValidateSignatureOptions): string | null => { - xml = sanitizeXML(xml); - return _hasValidSignature(xml, cert, certThumbprint, options); -}; - -const _hasValidSignature = (xml, cert, certThumbprint, options?: ValidateSignatureOptions): string | null => { const doc = parseFromString(xml); let signature = select( @@ -268,22 +259,28 @@ const _hasValidSignature = (xml, cert, certThumbprint, options?: ValidateSignatu return null; }; -// Breaking Change: validateSignature now returns: -// if signature is valid: the raw signed xml string -// if signature is invalid: throws error or returns null -// clients are to use the resultant raw xml string to parse their SAML Assertion - -const validateSignature = (xml, cert, certThumbprint, options?: ValidateSignatureOptions) => { +// Returns null when the signature does not verify, and throws when the +// document cannot be checked at all; validate() retries only on null. +const verifySignature = (xml, cert, certThumbprint, options?: ValidateSignatureOptions): string | null => { if (cert && certThumbprint) { throw new Error('You should provide either cert or certThumbprint, not both'); } - return hasValidSignature(xml, cert, certThumbprint, options); + return hasValidSignature(sanitizeXML(xml), cert, certThumbprint, options); +}; + +// Returns the signed XML, which is what callers must read from; throws when +// the signature does not verify. +const validateSignature = (xml, cert, certThumbprint, options?: ValidateSignatureOptions): string => { + const signedXml = verifySignature(xml, cert, certThumbprint, options); + if (!signedXml) { + throw new Error('invalid signature: the signature does not verify'); + } + return signedXml; }; const sanitizeXML = (xml) => { return xml.replace(/&#x(d|D);/gi, ''); }; -// certToPEM is re-exported for callers that import it from this module. -export { hasValidSignature, validateSignature, certToPEM, sanitizeXML }; +export { validateSignature, verifySignature, sanitizeXML }; diff --git a/package.json b/package.json index b760343c..1ee6b280 100644 --- a/package.json +++ b/package.json @@ -19,9 +19,16 @@ }, { "name": "Deepak Prabhakara", - "email": "deepak@boxyhq.com" + "email": "deepak.prabhakara@ory.sh" } ], + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + }, + "./package.json": "./package.json" + }, "main": "dist/index.js", "types": "dist/index.d.ts", "files": [ diff --git a/test/assets/certificates/oktaPublicKey.crt b/test/assets/certificates/oktaPublicKey.crt deleted file mode 100644 index 4328ba36..00000000 --- a/test/assets/certificates/oktaPublicKey.crt +++ /dev/null @@ -1,18 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIC3DCCAcSgAwIBAgIBATANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDEwd0ZXN0 -QXBwMBwXDTIyMDUxMDA5MDAyNloXCzEyMzExODMwMDBaMBIxEDAOBgNVBAMTB3Rl -c3RBcHAwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDPItnFnMX5NTNh -r77sEMLl6Ak5lmjv4BSPxL4mtA6Qj0h7NmFxGmo8+YG5qIznQfcIEDVRcaqrjsci -5X9MxahCKn830X1BrFaHWlshZ/5YzBH4mFKIscgH3MAOFLS88DhZQ81QIJ8NS1Rv -n3Adr+YJeSFg6MLjdpHQp9GqkjZ7bdIsXTWOQCV6HFVSxdhA3MQl4w4GgiGrugSc -guvrTd05drcsgOTf3a5fZk6S/AMY3rbuSAyNMVWhS6oOZUvxCZy9WWvyFCd9Rlqa -s2tjII9NN++eJBkqmJbBLw9uN34qLuD41XcDPp/qCDRKqBv5m+lxjFVGfDsHHOoA -42F4kH8fAgMBAAGjPzA9MAwGA1UdEwEB/wQCMAAwDgYDVR0PAQH/BAQDAgeAMB0G -A1UdDgQWBBRHQKMhTd5vQYNNluMk+ClGovjlXzANBgkqhkiG9w0BAQsFAAOCAQEA -VNvTH4opV4HuF2F900krOJUVVSzSKQvgp64CBmezgNMqrrx1Zu0fkYlfgicpIdn4 -/76AVekeQyZ179n9zRdesXwn9br4U3LAjfQXq9j5jXXUzJVFracIW27p48Y47v5h -Buotwocu8SIx8bXt8t5Jt3SL/z34zhTyxH1GR+d5XmG+4NRvaTXc1cRN+CXIcc5u -tynvwCmtjBWqqnLD0RsJP8nkc7eJnQ2GLAFhHI363eR0gt9GAq6/Oe5geBNzvIzK -PjXwC6+a55lxS9FwATCSvEgYa2IA/ls/P1Mv40VW+KGnMfu2sja8hvt5hOzsRzZX -ZPhP7PvmVbU6nQcDSI1V8w== ------END CERTIFICATE----- \ No newline at end of file diff --git a/test/assets/saml20.validAssertion.xml b/test/assets/saml20.validAssertion.xml deleted file mode 100644 index cda19fef..00000000 --- a/test/assets/saml20.validAssertion.xml +++ /dev/null @@ -1,67 +0,0 @@ - - - https://iamasamlissuer.com - - - - transient-subject - - - - - - - - - https://saml.boxyhq.com - - - - - - - - urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified - - - - - - - member@boxyhq.com - - - - - 123456789 - - - - - urn:mace:eduserv.org.uk:athens:federation:uk - - - - - member - - - - - 12345 - - - - - boxyhq.com - - - - - - permanent-id - - - - - diff --git a/test/lib/compat.spec.ts b/test/lib/compat.spec.ts index 51e7686d..e51400e1 100644 --- a/test/lib/compat.spec.ts +++ b/test/lib/compat.spec.ts @@ -1,138 +1,70 @@ import assert from 'assert'; -import { isMultiRootedXMLError } from '../../lib/utils'; -import { hasValidSignature } from '../../lib/validateSignature'; -import { ASSERTION_XPATH, buildAssertion, signXml, testIdpCert } from '../helpers/fixtures'; +import fs from 'fs'; +import path from 'path'; +import * as root from '../../lib/index'; -// Everything each module exported in 1.21.0. Consumers can import any built -// module directly (dist/), so removing one of these is a breaking change. -const published: Record = { - cert: { named: ['PubKeyInfo', 'stripCertHeaderAndFooter'] }, - decrypt: { named: ['decryptXml'] }, - getVersion: { named: ['getVersion'] }, - index: { - named: ['default'], - defaultKeys: [ - 'DEFAULT_MAX_SIGNATURE_REFERENCES', - 'MAX_ENCODED_REQUEST_LENGTH', - 'MAX_INFLATED_REQUEST_BYTES', - 'PubKeyInfo', - 'WrapError', - 'certToPEM', - 'containsDoctype', - 'createIdPMetadataXML', - 'createLogoutRequest', - 'createLogoutResponse', - 'createPostForm', - 'createSAMLResponse', - 'createSPMetadataXML', - 'decodeBase64', - 'decryptXml', - 'parse', - 'parseIssuer', - 'parseLogoutRequest', - 'parseLogoutResponse', - 'parseMetadata', - 'parseSAMLRequest', - 'request', - 'samlRequestDecodeError', - 'samlRequestTooLargeError', - 'sign', - 'stripCertHeaderAndFooter', - 'validate', - 'validateSignature', - ], - }, - logout: { - named: ['createLogoutRequest', 'createLogoutResponse', 'parseLogoutRequest', 'parseLogoutResponse'], - }, - metadata: { named: ['createIdPMetadataXML', 'createSPMetadataXML', 'parseMetadata'] }, - post: { named: ['createPostForm'] }, - request: { - named: [ - 'MAX_ENCODED_REQUEST_LENGTH', - 'MAX_INFLATED_REQUEST_BYTES', - 'decodeBase64', - 'parseSAMLRequest', - 'request', - 'samlRequestDecodeError', - 'samlRequestTooLargeError', - ], - }, - response: { named: ['WrapError', 'createSAMLResponse', 'parse', 'parseIssuer', 'validate'] }, - saml20: { - named: ['default'], - defaultKeys: [ - 'getAssertionId', - 'getDestination', - 'getInResponseTo', - 'getNotOnOrAfter', - 'getSubjectConfirmationInResponseTo', - 'parse', - 'validateAudience', - 'validateExpiration', - 'validateRecipient', - ], - }, - sign: { named: ['sign'] }, - typings: { named: [] }, - utils: { - named: [ - 'containsDoctype', - 'doctypeNotAllowedError', - 'getAttribute', - 'isMultiRootedXMLError', - 'multiRootedXMLError', - 'parseFromString', - 'thumbprint', - ], - }, - validateSignature: { - named: [ - 'DEFAULT_MAX_SIGNATURE_REFERENCES', - 'certToPEM', - 'hasValidSignature', - 'sanitizeXML', - 'validateSignature', - ], - }, -}; +const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, '../../package.json'), 'utf8')); -describe('published module exports', function () { - for (const [name, { named, defaultKeys }] of Object.entries(published)) { - it(`lib/${name} still exports everything 1.21.0 did`, function () { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const mod = require(`../../lib/${name}`); - assert.deepStrictEqual( - named.filter((key) => mod[key] === undefined), - [], - 'missing named exports' - ); - if (defaultKeys) { - assert.deepStrictEqual( - defaultKeys.filter((key) => mod.default?.[key] === undefined), - [], - 'missing keys on the default export' - ); - } - }); - } -}); +// The package root is the whole public API: adding to this list is a feature, +// removing from it is a breaking change. +const api = [ + 'DEFAULT_MAX_SIGNATURE_REFERENCES', + 'MAX_ENCODED_REQUEST_LENGTH', + 'MAX_INFLATED_REQUEST_BYTES', + 'WrapError', + 'certToPEM', + 'containsDoctype', + 'createIdPMetadataXML', + 'createLogoutRequest', + 'createLogoutResponse', + 'createPostForm', + 'createSAMLResponse', + 'createSPMetadataXML', + 'decodeBase64', + 'parseIssuer', + 'parseLogoutRequest', + 'parseLogoutResponse', + 'parseMetadata', + 'parseSAMLRequest', + 'request', + 'samlRequestDecodeError', + 'samlRequestTooLargeError', + 'sign', + 'stripCertHeaderAndFooter', + 'validate', + 'validateSignature', +]; -describe('deprecated module exports keep working', function () { - it('utils.isMultiRootedXMLError recognises the parser message', function () { - assert.strictEqual( - isMultiRootedXMLError({ message: 'Only one element can be added and only after doctype' }), - true +describe('public API', function () { + it('the package root has exactly these named exports', function () { + assert.deepStrictEqual( + Object.keys(root) + .filter((key) => key !== 'default') + .sort(), + api ); - assert.strictEqual(isMultiRootedXMLError({ message: 'Other error' }), false); }); - it('validateSignature.hasValidSignature verifies a signature', function () { - const signed = signXml(buildAssertion(), ASSERTION_XPATH); - assert(hasValidSignature(signed, testIdpCert, null)); - assert.strictEqual( - hasValidSignature(signed.replace('jo@example.com', 'eve@example.com'), testIdpCert, null), - null - ); + it('the default export has exactly the same keys', function () { + assert.deepStrictEqual(Object.keys(root.default).sort(), api); + }); + + it('package.json exports only the package root', function () { + assert.deepStrictEqual(pkg.exports, { + '.': { types: './dist/index.d.ts', default: './dist/index.js' }, + './package.json': './package.json', + }); + assert.strictEqual(pkg.exports['.'].types, `./${pkg.types}`); + assert.strictEqual(pkg.exports['.'].default, `./${pkg.main}`); + }); + + it('a module inside the package cannot be imported', function () { + // Resolved through the package's own name, so Node applies the exports map. + assert.throws(() => require.resolve(`${pkg.name}/dist/typings`), { + code: 'ERR_PACKAGE_PATH_NOT_EXPORTED', + }); + assert.throws(() => require.resolve(`${pkg.name}/dist/validateSignature`), { + code: 'ERR_PACKAGE_PATH_NOT_EXPORTED', + }); }); }); diff --git a/test/lib/decrypt.response.spec.ts b/test/lib/decrypt.response.spec.ts index 80c74080..5fcf77a7 100644 --- a/test/lib/decrypt.response.spec.ts +++ b/test/lib/decrypt.response.spec.ts @@ -8,7 +8,6 @@ const oneLoginSamlResponseEncrypted = fs const oneLoginPrivateKey = fs.readFileSync('./test/assets/certificates/oneloginPrivateKey.pem').toString(); const oneLoginThumbprint = '56d68c4616d0909ac25dade25c36a7bd792eaf62'; const oneLoginInResponseTo = '_25b63bdecac84d524aec'; -// const oneLoginCertificate = fs.readFileSync('./test/assets/certificates/oneloginPublicKey.crt').toString(); const oneLoginIssuerName = 'https://app.onelogin.com/saml/metadata/2f5926c1-a571-4702-9ed5-12309c86f9c7'; const oneLoginProfileClaims = 'hojit22291@abincol.com'; @@ -18,13 +17,15 @@ const oktaSamlResponseEncrypted = fs const oktaPrivateKey = fs.readFileSync('./test/assets/certificates/oktaPrivateKey.pem').toString(); const oktaThumbprint = '008c1aa2ed3cdb5c064c99b3d1619346b619008a'; const oktaInResponseTo = '_f81f46f19ccf489ab1a1'; -// const oktaCertificate = fs.readFileSync('./test/assets/certificates/oktaPublicKey.crt').toString(); const oktaIssuerName = 'http://www.okta.com/exkymhf9ve6PI9KfY696'; const oktaProfileClaims = 'hojit22291@abincol.com'; +const samlAudience = 'https://saml.boxyhq.com'; + describe('decrypt.response.spec', function () { it('One Login Should validate saml 2.0 token using thumbprint', async function () { const response = await validate(oneLoginSamlResponseEncrypted, { + audience: samlAudience, privateKey: oneLoginPrivateKey, thumbprint: oneLoginThumbprint, bypassExpiration: true, @@ -40,6 +41,7 @@ describe('decrypt.response.spec', function () { it('One Login Should validate saml 2.0 token using thumbprint Only', async function () { const response = await validate(oneLoginSamlResponseEncrypted, { + audience: samlAudience, privateKey: oneLoginPrivateKey, thumbprint: oneLoginThumbprint, bypassExpiration: true, @@ -54,6 +56,7 @@ describe('decrypt.response.spec', function () { it('Okta Should validate saml 2.0 token using thumbprint', async function () { const response = await validate(oktaSamlResponseEncrypted, { + audience: samlAudience, privateKey: oktaPrivateKey, thumbprint: oktaThumbprint, bypassExpiration: true, @@ -68,6 +71,7 @@ describe('decrypt.response.spec', function () { it('Okta Should validate saml 2.0 token using thumbprint Only. Also test multiple thumprint validation.', async function () { const response = await validate(oktaSamlResponseEncrypted, { + audience: samlAudience, privateKey: oktaPrivateKey, thumbprint: `${oktaThumbprint},somedummythumbprint`, bypassExpiration: true, diff --git a/test/lib/decrypt.spec.ts b/test/lib/decrypt.spec.ts index c494bc84..886a163c 100644 --- a/test/lib/decrypt.spec.ts +++ b/test/lib/decrypt.spec.ts @@ -105,7 +105,11 @@ describe('decrypt.ts', function () { ); assert.throws(() => decryptXml(response, { privateKey: testIdpPrivateKey }), multiRootedXMLError); await assert.rejects( - validate(response, { privateKey: testIdpPrivateKey, publicKey: testIdpCert }), + validate(response, { + audience: 'https://sp.example.com/metadata', + privateKey: testIdpPrivateKey, + publicKey: testIdpCert, + }), multiRootedXMLError ); }); diff --git a/test/lib/getVersion.spec.ts b/test/lib/getVersion.spec.ts index 4a399894..b0d31c72 100644 --- a/test/lib/getVersion.spec.ts +++ b/test/lib/getVersion.spec.ts @@ -1,6 +1,5 @@ import assert from 'assert'; import * as version from '../../lib/getVersion'; -import { parse } from '../../lib/response'; describe('getVersion.ts', function () { it('getVersion not ok', function () { @@ -17,13 +16,3 @@ describe('getVersion without attributes', function () { assert.strictEqual(version.getVersion({ Issuer: 'https://idp' }), null); }); }); - -describe('parse() with an Assertion that has no Version', function () { - it('rejects with "version not supported" instead of hanging', async function () { - const response = - '' + - 'https://idp'; - await assert.rejects(parse(response), { message: 'SAML Assertion version not supported.' }); - }); -}); diff --git a/test/lib/index.spec.ts b/test/lib/index.spec.ts index a369e22b..fe9f8c5d 100644 --- a/test/lib/index.spec.ts +++ b/test/lib/index.spec.ts @@ -29,7 +29,6 @@ describe('exported types', function () { const options: ValidateOptions = { publicKey: 'MIIC...', audience: ['https://sp.example.com', 'https://sp2.example.com'], - strictAudienceValidation: true, inResponseTo: '_request', recipient: 'https://sp.example.com/acs', allowedSignatureAlgorithms: ['http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'], @@ -40,6 +39,10 @@ describe('exported types', function () { return used; }, }; + // @ts-expect-error strictAudienceValidation was removed in 2.0; audiences always match exactly + const removed: ValidateOptions = { publicKey: 'MIIC...', strictAudienceValidation: true }; + // @ts-expect-error audience is required + const noAudience: ValidateOptions = { publicKey: 'MIIC...' }; // @ts-expect-error audience must be a string or a list of strings const invalid: ValidateOptions = { publicKey: 'MIIC...', audience: 42 }; const profile: SAMLProfile = { @@ -53,7 +56,7 @@ describe('exported types', function () { assert.strictEqual(await options.assertionReplayValidator!({ assertionId: '_a' }), false); assert.strictEqual(await options.assertionReplayValidator!({ assertionId: '_a' }), true); - assert(invalid && profile && request); + assert(removed && noAudience && invalid && profile && request); }); }); diff --git a/test/lib/response.spec.ts b/test/lib/response.spec.ts index 3856ea5a..3e2f23b7 100644 --- a/test/lib/response.spec.ts +++ b/test/lib/response.spec.ts @@ -1,11 +1,16 @@ import assert from 'assert'; -import { parse, parseIssuer, validate, createSAMLResponse, WrapError } from '../../lib/response'; +import { parseIssuer, validate, createSAMLResponse, WrapError } from '../../lib/response'; +import type { ValidateOptions } from '../../lib/typings'; import { doctypeNotAllowedError } from '../../lib/utils'; import saml20 from '../../lib/saml20'; import fs from 'fs'; +import { SignedXml } from 'xml-crypto'; import { ACS, + ASSERTION_NS, + ASSERTION_XPATH, IDP, + PROTOCOL_NS, REQUEST_ID, RESPONSE_XPATH, SP, @@ -37,59 +42,39 @@ const suffixAudience = 'http://demoscope'; const validToken = fs.readFileSync('./test/assets/saml20.validToken.xml').toString(); const invalidToken = fs.readFileSync('./test/assets/saml20.invalidToken.xml').toString(); const invalidWrappedToken = fs.readFileSync('./test/assets/saml20.invalidWrappedToken.xml').toString(); -const validAssertion = fs.readFileSync('./test/assets/saml20.validAssertion.xml').toString(); -const oktaPublicKey = fs.readFileSync('./test/assets/certificates/oktaPublicKey.crt').toString(); -const oktaPrivateKey = fs.readFileSync('./test/assets/certificates/oktaPrivateKey.pem').toString(); const testIdpPublicKey = fs.readFileSync('./test/assets/certificates/testIdpCert.crt').toString(); const testIdpPrivateKey = fs.readFileSync('./test/assets/certificates/testIdpKey.pem').toString(); describe('response.ts', function () { - it('RAW response ok', async function () { - const response = await parse(rawResponse); - assert.strictEqual(response.audience, 'http://sp.example.com/demo1/metadata.php'); - assert.strictEqual( - response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], - '_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7' - ); - assert.strictEqual(response.issuer, 'http://idp.example.com/metadata.php'); - }); - - it('RAW response with invalid StatusCode', async function () { - await assert.rejects(parse(rawResponseAuthnFailed), { message: 'Invalid Status Code (AuthnFailed).' }); + it('rejects a response with an unsuccessful StatusCode', async function () { + await assert.rejects(validate(rawResponseAuthnFailed, validateOpts), { + message: 'Invalid Status Code (AuthnFailed).', + }); }); - it('RAW response not ok', async function () { - try { - await parse('rawResponse'); - } catch (error) { - const result = (error as Error).message; - assert.strictEqual(result, 'An error occurred trying to parse XML assertion.'); - } + it('rejects a response that is not XML', async function () { + await assert.rejects(validate('rawResponse', validateOpts), { message: 'missing root element' }); }); - // xml2js runs on the assertion after xmldom, so it needs its own DTD guard. // See https://github.com/ory/polis/issues/4071. - it('RAW response with a DTD is rejected before xml2js', async function () { + it('rejects a response with a DTD', async function () { const dtdResponse = '\n' + ' ]>\n' + '&x;'; - await assert.rejects(parse(dtdResponse), (err: Error & { inner?: unknown }) => { - assert.strictEqual(err.inner, doctypeNotAllowedError); - return true; - }); + await assert.rejects(validate(dtdResponse, validateOpts), doctypeNotAllowedError); }); // Rejecting the DTD must not leave a detached async continuation that // dereferences undefined and crashes the process. See ory/polis#4071. - it('should not emit an unhandled rejection when parse() rejects a DTD', async function () { + it('should not emit an unhandled rejection when validate() rejects a DTD', async function () { const unhandled: unknown[] = []; const onUnhandled = (reason: unknown) => unhandled.push(reason); process.on('unhandledRejection', onUnhandled); try { await assert.rejects( - parse(' ]>ok') + validate(' ]>ok', validateOpts) ); // Let any detached continuation settle before asserting. await new Promise((resolve) => setImmediate(resolve)); @@ -100,37 +85,10 @@ describe('response.ts', function () { } }); - it('Should not parse saml 2.0 token which has no assertion', async function () { - try { - await parse(errorResponse); - } catch (error) { - const result = (error as Error).message; - assert.strictEqual(result, 'Invalid Status Code (AuthnFailed).'); - } - }); - - it('An error occurred trying to parse XML assertion.', async function () { - try { - await parse('undefined'); - } catch (error) { - const result = (error as Error).message; - assert.strictEqual(result, 'An error occurred trying to parse XML assertion.'); - } - }); - - it('An error occurred trying to parse assertion', async function () { - try { - const response = await parse(validResponse); - assert.strictEqual(response.audience, 'http://sp.example.com/demo1/metadata.php'); - assert.strictEqual( - response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], - '_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7' - ); - assert.strictEqual(response.issuer, 'http://idp.example.com/metadata.php'); - } catch (error) { - const result = (error as Error).message; - assert.strictEqual(result, 'An error occurred trying to parse assertion.'); - } + it('rejects an error response that has no assertion', async function () { + await assert.rejects(validate(errorResponse, { audience: SP, publicKey: testIdpCert }), { + message: 'Invalid Status Code (AuthnFailed).', + }); }); it('validate ok', async function () { @@ -197,6 +155,7 @@ describe('response.ts', function () { it('Should fail which has no assertion', async function () { try { await validate(validResponse, { + audience: validateOpts.audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -210,6 +169,7 @@ describe('response.ts', function () { it('Should fail which has no publicKey or thumbprint', async function () { try { await validate(validResponse, { + audience: validateOpts.audience, publicKey: undefined, bypassExpiration: true, inResponseTo: inResponseTo, @@ -222,6 +182,7 @@ describe('response.ts', function () { it('Assertion is expired.', async function () { try { validate('invalid-assertion', { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -233,7 +194,7 @@ describe('response.ts', function () { }); it('Should validate saml 2.0 token using certificate', async function () { - const response = await validate(validToken, { publicKey: certificate, bypassExpiration: true }); + const response = await validate(validToken, { audience, publicKey: certificate, bypassExpiration: true }); assert.strictEqual(response.issuer, issuerName); assert.strictEqual( response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'], @@ -250,27 +211,11 @@ describe('response.ts', function () { assert.strictEqual(response.audience, audience); }); - it('Should validate saml 2.0 token and check audience with suffix', async function () { - const response = await validate(validToken, { - publicKey: certificate, - audience: suffixAudience, - bypassExpiration: true, - }); - assert.strictEqual(response.audience, audience); - }); - - it('Should validate saml 2.0 token and check strict audience with suffix', async function () { - try { - await validate(validToken, { - publicKey: certificate, - audience: suffixAudience, - bypassExpiration: true, - strictAudienceValidation: true, - }); - } catch (error) { - const result = (error as Error).message; - assert.strictEqual(result, 'Invalid audience.'); - } + it('Should reject saml 2.0 token when the expected audience is only a prefix', async function () { + await assert.rejects( + validate(validToken, { publicKey: certificate, audience: suffixAudience, bypassExpiration: true }), + { message: 'Invalid audience.' } + ); }); it('Should fail with invalid audience', async function () { @@ -287,7 +232,7 @@ describe('response.ts', function () { }); it('Should fail with invalid signature', async function () { try { - await validate(invalidToken, { publicKey: certificate, bypassExpiration: true }); + await validate(invalidToken, { audience, publicKey: certificate, bypassExpiration: true }); } catch (error) { const result = (error as Error).message; assert.strictEqual(result, 'Invalid assertion signature.'); @@ -296,7 +241,7 @@ describe('response.ts', function () { it('Should fail with missing root element', async function () { try { - await validate('invalid-assertion', { publicKey: certificate, bypassExpiration: true }); + await validate('invalid-assertion', { audience, publicKey: certificate, bypassExpiration: true }); } catch (error) { const result = (error as Error).message; assert.strictEqual(result, 'missing root element'); @@ -305,7 +250,7 @@ describe('response.ts', function () { it('Should fail with invalid assertion and possible assertion wrapping', async function () { try { - await validate(invalidWrappedToken, { publicKey: certificate, bypassExpiration: true }); + await validate(invalidWrappedToken, { audience, publicKey: certificate, bypassExpiration: true }); } catch (error) { const result = (error as Error).message; assert.strictEqual(result, 'Invalid assertion. Possible assertion wrapping.'); @@ -314,18 +259,13 @@ describe('response.ts', function () { it('Should fail with expired assertion', async function () { try { - await validate(validToken, { publicKey: certificate }); + await validate(validToken, { audience, publicKey: certificate }); } catch (error) { const result = (error as Error).message; assert.strictEqual(result, 'Assertion is expired.'); } }); - it('Should parse saml 2.0 without signature validation', async function () { - const response = await parse(invalidToken); - assert.strictEqual(response.issuer, issuerName); - }); - it('parseIssuer response ok', async function () { const issuer = await parseIssuer(validResponse); assert.strictEqual(issuer, 'http://idp.example.com/metadata.php'); @@ -339,10 +279,21 @@ describe('response.ts', function () { } }); - it('Should parse saml 2.0 assertion and check nameidentifier picks up nameid-permanent', async function () { - const response = await parse(validAssertion); + it('maps a persistent NameID attribute value to the nameidentifier claim', async function () { + const response = signXml( + buildResponse( + buildAssertion({ + attributes: + '' + + 'permanent-id' + + '', + }) + ), + RESPONSE_XPATH + ); + const profile = await validate(response, { audience: SP, publicKey: testIdpCert }); assert.strictEqual( - response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], + profile.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], 'permanent-id' ); }); @@ -361,13 +312,13 @@ describe('response.ts', function () { }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, }; const response = await createSAMLResponse(json); - const parsed = await parse(response); + const parsed = await validate(response, { audience: json.audience, publicKey: testIdpPublicKey }); assert.strictEqual(parsed.issuer, json.issuer); assert.strictEqual(parsed.audience, json.audience); @@ -389,14 +340,14 @@ describe('response.ts', function () { }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, flattenArray: true, }; const response = await createSAMLResponse(json); - const parsed = await parse(response); + const parsed = await validate(response, { audience: json.audience, publicKey: testIdpPublicKey }); assert.strictEqual(parsed.issuer, json.issuer); assert.strictEqual(parsed.audience, json.audience); @@ -415,12 +366,15 @@ describe('response.ts', function () { }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, sessionIndex: '_session-index-from-the-idp', }; - const parsed = await parse(await createSAMLResponse(json)); + const parsed = await validate(await createSAMLResponse(json), { + audience: json.audience, + publicKey: testIdpPublicKey, + }); assert.strictEqual(parsed.sessionIndex, json.sessionIndex); }); @@ -471,8 +425,8 @@ it('Should create a SAML response with nameFormat basic', async function () { }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, }; const response = await createSAMLResponse(json); @@ -488,6 +442,7 @@ it('Should create a SAML response with nameFormat basic', async function () { it('validate should throw error if both publicKey and thumbprint are provided', async function () { try { await validate(validResponse, { + audience: validateOpts.audience, publicKey: certificate, thumbprint: 'thumbprint', }); @@ -501,15 +456,7 @@ it('validate should throw error if both publicKey and thumbprint are provided', it('validate should throw error if rawAssertion is empty', async function () { try { - await validate('', { publicKey: certificate }); - } catch (error) { - assert.strictEqual((error as Error).message, 'rawAssertion is required.'); - } -}); - -it('parse should throw error if rawAssertion is empty', async function () { - try { - await parse(''); + await validate('', { audience: SP, publicKey: certificate }); } catch (error) { assert.strictEqual((error as Error).message, 'rawAssertion is required.'); } @@ -527,8 +474,8 @@ it('Should create a SAML response with default ttlInMinutes', async function () }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, }; const response = await createSAMLResponse(json); @@ -561,8 +508,8 @@ it('Should create a SAML response with custom ttlInMinutes', async function () { }, }, requestId: 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685', - privateKey: oktaPrivateKey, - publicKey: oktaPublicKey, + privateKey: testIdpPrivateKey, + publicKey: testIdpPublicKey, ttlInMinutes, }; @@ -638,27 +585,140 @@ describe('validate() with an assertion encrypted before the Response was signed' }); }); -describe('parse() edge cases', function () { - it('rejects a Response without an Assertion', async function () { - await assert.rejects(parse(buildResponse('')), { message: 'Invalid assertion.' }); +describe('validate() with a WS-Trust RequestSecurityTokenResponse (ADFS over WS-Federation)', function () { + const rstr = (token: string) => + '' + + '' + + new Date().toISOString() + + '' + + `${SP}` + + `${token}` + + ''; + + // An enveloped signature over the whole RequestSecurityTokenResponse. + const signEnvelope = (xml: string) => { + const sig = new SignedXml({ + privateKey: testIdpKey, + signatureAlgorithm: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256', + canonicalizationAlgorithm: 'http://www.w3.org/2001/10/xml-exc-c14n#', + }); + sig.addReference({ + xpath: '/*', + transforms: [ + 'http://www.w3.org/2000/09/xmldsig#enveloped-signature', + 'http://www.w3.org/2001/10/xml-exc-c14n#', + ], + digestAlgorithm: 'http://www.w3.org/2001/04/xmlenc#sha256', + }); + sig.computeSignature(xml, { location: { reference: '/*', action: 'append' } }); + return sig.getSignedXml(); + }; + + it('accepts a signed Assertion inside an unsigned RequestSecurityTokenResponse', async function () { + const assertion = signXml(buildAssertion(), ASSERTION_XPATH).replace(/^<\?xml[^>]*>/, ''); + const profile = await validate(rstr(assertion), { publicKey: testIdpCert, audience: SP }); + assert.strictEqual(profile.issuer, IDP); + assert.strictEqual(profile.assertionId, '_assertion-1'); + }); + + it('accepts an Assertion inside a signed RequestSecurityTokenResponse', async function () { + const profile = await validate(signEnvelope(rstr(buildAssertion())), { + publicKey: testIdpCert, + audience: SP, + }); + assert.strictEqual(profile.issuer, IDP); + assert.strictEqual(profile.assertionId, '_assertion-1'); }); - it('uses the first Assertion when a Response carries several', async function () { - const response = buildResponse(buildAssertion({ id: '_first' }) + buildAssertion({ id: '_second' })); - const profile = await parse(response); + it('rejects a signed RequestSecurityTokenResponse whose Assertion was changed', async function () { + const tampered = signEnvelope(rstr(buildAssertion())).replace('jo@example.com', 'eve@example.com'); + await assert.rejects(validate(tampered, { publicKey: testIdpCert, audience: SP }), { + message: 'Invalid assertion signature.', + }); + }); + + it('runs the usual checks on the Assertion inside a signed RequestSecurityTokenResponse', async function () { + await assert.rejects( + validate(signEnvelope(rstr(buildAssertion())), { + publicKey: testIdpCert, + audience: 'https://other.example.com', + }), + { message: 'Invalid audience.' } + ); + }); +}); + +describe('validate() edge cases', function () { + it('uses the first Assertion when a signed Response carries several', async function () { + const response = signXml( + buildResponse(buildAssertion({ id: '_first' }) + buildAssertion({ id: '_second' })), + RESPONSE_XPATH + ); + const profile = await validate(response, { publicKey: testIdpCert, audience: SP }); assert.strictEqual(profile.assertionId, '_first'); }); + // sanitizeXML strips after the DOM parser's DTD check, so it can + // assemble a DTD keyword that only the xml2js guard sees. + it('rejects a DTD keyword that sanitizeXML assembles before xml2js', async function () { + for (const inner of ['', ']]>']) { + await assert.rejects( + validate(buildResponse(inner), { publicKey: testIdpCert, audience: SP }), + (error: WrapError) => { + assert.strictEqual(error.message, 'An error occurred trying to parse XML assertion.'); + assert.strictEqual(error.inner, doctypeNotAllowedError); + return true; + } + ); + } + }); + + it('wraps a document that the DOM parser accepts but xml2js rejects', async function () { + await assert.rejects( + validate(buildResponse('a & b'), { publicKey: testIdpCert, audience: SP }), + (error: WrapError) => { + assert(error instanceof WrapError); + assert.strictEqual(error.message, 'An error occurred trying to parse XML assertion.'); + assert.match((error.inner as Error).message, /Invalid character in entity name/); + return true; + } + ); + }); + + it('rejects a signed Response without an Assertion', async function () { + await assert.rejects( + validate(signXml(buildResponse(''), RESPONSE_XPATH), { publicKey: testIdpCert, audience: SP }), + { + message: 'Invalid assertion.', + } + ); + }); + + it('rejects a signed Assertion that has no Version', async function () { + const response = signXml( + `` + + `${IDP}` + + `${IDP}`, + RESPONSE_XPATH + ); + await assert.rejects(validate(response, { audience: SP, publicKey: testIdpCert }), { + message: 'SAML Assertion version not supported.', + }); + }); + it('wraps a failure while reading attributes', async function () { // An without a Name has no attributes at all, so reading its // Name throws inside the claims parser. - const response = buildResponse( - buildAssertion({ - attributes: - 'x', - }) + const response = signXml( + buildResponse( + buildAssertion({ + attributes: + 'x', + }) + ), + RESPONSE_XPATH ); - await assert.rejects(parse(response), (error: WrapError) => { + await assert.rejects(validate(response, { audience: SP, publicKey: testIdpCert }), (error: WrapError) => { assert(error instanceof WrapError); assert.strictEqual(error.message, 'An error occurred trying to parse assertion.'); assert(error.inner instanceof Error); @@ -667,10 +727,65 @@ describe('parse() edge cases', function () { }); }); +describe('validate() requires an audience', function () { + const signed = () => signXml(buildResponse(buildAssertion()), RESPONSE_XPATH); + const missing: [string, unknown][] = [ + ['omitted', undefined], + ['an empty string', ''], + ['an empty list', []], + ['a list with an empty string', [SP, '']], + ['not a string', 42], + ]; + + for (const [name, audience] of missing) { + it(`rejects an audience that is ${name}`, async function () { + await assert.rejects( + validate(signed(), { publicKey: testIdpCert, audience } as unknown as ValidateOptions), + { message: 'audience is required.' } + ); + }); + } +}); + +describe('validate() matches audiences exactly', function () { + const signedFor = (audience: string) => + signXml(buildResponse(buildAssertion({ audienceRestrictions: [[audience]] })), RESPONSE_XPATH); + + it('accepts an assertion whose Audience equals the expected one', async function () { + const profile = await validate(signedFor(SP), { publicKey: testIdpCert, audience: SP }); + assert.strictEqual(profile.audience, SP); + }); + + // A prefix match let an assertion issued for https://sp.example.com/metadata.evil.example + // pass as one for https://sp.example.com/metadata. + it('rejects an assertion whose Audience only starts with the expected one', async function () { + await assert.rejects( + validate(signedFor(`${SP}.evil.example`), { publicKey: testIdpCert, audience: SP }), + { + message: 'Invalid audience.', + } + ); + }); + + it('rejects an assertion whose Audience is a prefix of the expected one', async function () { + await assert.rejects( + validate(signedFor('https://sp.example.com'), { publicKey: testIdpCert, audience: SP }), + { message: 'Invalid audience.' } + ); + }); + + it('rejects an expected audience that is a prefix of one in a list', async function () { + await assert.rejects( + validate(signedFor(`${SP}/x`), { publicKey: testIdpCert, audience: ['https://z.example.com', SP] }), + { message: 'Invalid audience.' } + ); + }); +}); + describe('validate() audience checks on signed responses', function () { const signedWith = (audienceRestrictions: string[][]) => signXml(buildResponse(buildAssertion({ audienceRestrictions })), RESPONSE_XPATH); - const options = { publicKey: testIdpCert, audience: SP, strictAudienceValidation: true }; + const options = { publicKey: testIdpCert, audience: SP }; it('accepts an assertion that lists the SP among several audiences', async function () { const profile = await validate(signedWith([['https://other.example.com', SP]]), options); @@ -708,7 +823,7 @@ describe('validate() audience checks on signed responses', function () { describe('validate() on an unsigned response', function () { it('rejects it and keeps the reason in inner', async function () { await assert.rejects( - validate(buildResponse(buildAssertion()), { publicKey: testIdpCert }), + validate(buildResponse(buildAssertion()), { audience: SP, publicKey: testIdpCert }), (error: WrapError) => { assert.strictEqual(error.message, 'Invalid assertion.'); assert.strictEqual(error.inner.message, 'invalid signature: no Signature element found'); @@ -727,12 +842,15 @@ describe('validate() error handling', function () { throw new Error('boom'); }; try { - await assert.rejects(validate(signedResponse(), { publicKey: testIdpCert }), (error: WrapError) => { - assert(error instanceof WrapError); - assert.strictEqual(error.message, 'An error occurred trying to validate the assertion.'); - assert.strictEqual(error.inner.message, 'boom'); - return true; - }); + await assert.rejects( + validate(signedResponse(), { audience: SP, publicKey: testIdpCert }), + (error: WrapError) => { + assert(error instanceof WrapError); + assert.strictEqual(error.message, 'An error occurred trying to validate the assertion.'); + assert.strictEqual(error.inner.message, 'boom'); + return true; + } + ); } finally { saml20.validateExpiration = original; } @@ -741,6 +859,7 @@ describe('validate() error handling', function () { it('wraps a failing replay validator and keeps its error in inner', async function () { await assert.rejects( validate(signedResponse(), { + audience: SP, publicKey: testIdpCert, assertionReplayValidator: async () => { throw new Error('store unavailable'); @@ -761,21 +880,22 @@ describe('validate() error handling', function () { try { const signed = signedResponse(); const attempts: [string, Record][] = [ - ['', { publicKey: testIdpCert }], + ['', { publicKey: testIdpCert, audience: SP }], [signed, {}], - [signed, { publicKey: testIdpCert, thumbprint: 'x' }], - ['', { publicKey: testIdpCert }], - [buildResponse(''), { publicKey: testIdpCert }], - [buildResponse(buildAssertion()), { publicKey: testIdpCert }], - [signed.replace('jo@example.com', 'eve@example.com'), { publicKey: testIdpCert }], - [signed, { publicKey: certificate }], + [signed, { publicKey: testIdpCert }], + [signed, { publicKey: testIdpCert, thumbprint: 'x', audience: SP }], + ['', { publicKey: testIdpCert, audience: SP }], + [buildResponse(''), { publicKey: testIdpCert, audience: SP }], + [buildResponse(buildAssertion()), { publicKey: testIdpCert, audience: SP }], + [signed.replace('jo@example.com', 'eve@example.com'), { publicKey: testIdpCert, audience: SP }], + [signed, { publicKey: certificate, audience: SP }], [signed, { publicKey: testIdpCert, audience: 'https://other.example.com' }], - [signed, { publicKey: testIdpCert, inResponseTo: '_other' }], - [signed, { publicKey: testIdpCert, recipient: 'https://other.example.com/acs' }], - [signed, { publicKey: testIdpCert, assertionReplayValidator: () => true }], + [signed, { publicKey: testIdpCert, audience: SP, inResponseTo: '_other' }], + [signed, { publicKey: testIdpCert, audience: SP, recipient: 'https://other.example.com/acs' }], + [signed, { publicKey: testIdpCert, audience: SP, assertionReplayValidator: () => true }], ]; for (const [xml, options] of attempts) { - await assert.rejects(validate(xml, options)); + await assert.rejects(validate(xml, options as unknown as ValidateOptions)); } await new Promise((resolve) => setTimeout(resolve, 50)); assert.deepStrictEqual(unhandled, []); diff --git a/test/lib/saml20.response.spec.ts b/test/lib/saml20.response.spec.ts index 4020dfe1..1a17f202 100644 --- a/test/lib/saml20.response.spec.ts +++ b/test/lib/saml20.response.spec.ts @@ -1,6 +1,7 @@ import assert from 'assert'; import { validate } from '../../lib/response'; import fs from 'fs'; +import { SignedXml } from 'xml-crypto'; // Tests Configuration const digestValueComment = fs.readFileSync('./test/assets/digestValueComment.xml').toString(); @@ -20,6 +21,7 @@ const inResponseTo = 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'; describe('lib.saml20.response', function () { it('Should validate saml 2.0 token using thumbprint', async function () { const response = await validate(validResponse, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -33,6 +35,7 @@ describe('lib.saml20.response', function () { it('Should validate saml 2.0 token using certificate', async function () { const response = await validate(validResponse, { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -47,10 +50,12 @@ describe('lib.saml20.response', function () { it('Should honour algorithm allowlists passed to validate()', async function () { // saml20.validResponse.xml is signed with RSA-SHA1 / SHA-1 const sha1Options = { + audience, allowedSignatureAlgorithms: ['http://www.w3.org/2000/09/xmldsig#rsa-sha1'], allowedHashAlgorithms: ['http://www.w3.org/2000/09/xmldsig#sha1'], }; const sha2Options = { + audience, allowedSignatureAlgorithms: ['http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'], allowedHashAlgorithms: ['http://www.w3.org/2001/04/xmlenc#sha256'], }; @@ -80,6 +85,7 @@ describe('lib.saml20.response', function () { // thumbprint path await assert.rejects( validate(validResponse, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -99,7 +105,12 @@ describe('lib.saml20.response', function () { const inflated = validResponse.replace(match![0], match![0].repeat(1500)); await assert.rejects( - validate(inflated, { publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo }), + validate(inflated, { + audience, + publicKey: certificate, + bypassExpiration: true, + inResponseTo: inResponseTo, + }), (err: any) => { assert.strictEqual(err.message, 'Invalid assertion.'); assert.match(err.inner.message, /SignedInfo declares 1500 References; at most 16 are allowed/); @@ -113,6 +124,7 @@ describe('lib.saml20.response', function () { const seventeen = validResponse.replace(match![0], match![0].repeat(17)); await assert.rejects( validate(seventeen, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -125,6 +137,35 @@ describe('lib.saml20.response', function () { ); }); + it('Should not canonicalize any Reference of an inflated SignedInfo in validate()', async function () { + const match = validResponse.match(/<(?:[\w.-]+:)?Reference\b[\s\S]*?<\/(?:[\w.-]+:)?Reference>/); + assert(match, 'fixture carries a Reference to duplicate'); + const options = { audience, publicKey: certificate, bypassExpiration: true, inResponseTo }; + + // getCanonReferenceXml is xml-crypto's per-Reference canonicalization, the + // work the Reference ceiling exists to bound. + const proto = SignedXml.prototype as unknown as { getCanonReferenceXml: (...args: unknown[]) => unknown }; + const original = proto.getCanonReferenceXml; + let calls = 0; + proto.getCanonReferenceXml = function (...args: unknown[]) { + calls++; + return original.apply(this, args); + }; + try { + await assert.rejects(validate(validResponse.replace(match![0], match![0].repeat(1500)), options), { + message: 'Invalid assertion.', + }); + assert.strictEqual(calls, 0, 'no Reference was canonicalized before the count check'); + + // The same spy sees the single Reference of the unmodified response, so + // the zero above is not vacuous. + await validate(validResponse, options); + assert(calls > 0, 'the spy is wired to the xml-crypto instance validate() uses'); + } finally { + proto.getCanonReferenceXml = original; + } + }); + it('Should validate saml 2.0 token and check audience', async function () { const response = await validate(validResponse, { publicKey: certificate, @@ -147,6 +188,7 @@ describe('lib.saml20.response', function () { it('Should validate unsanitized saml 2.0 token using thumbprint', async function () { const response = await validate(validResponseUnsanitized, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -160,6 +202,7 @@ describe('lib.saml20.response', function () { it('Should validate unsanitized saml 2.0 token using certificate', async function () { const response = await validate(validResponseUnsanitized, { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -188,6 +231,7 @@ describe('lib.saml20.response', function () { it('Should fail with missing root element', async function () { try { await validate('invalid-assertion', { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, diff --git a/test/lib/saml20.responseSignedMessage.spec.ts b/test/lib/saml20.responseSignedMessage.spec.ts index 3b2e4d27..3badbfd7 100644 --- a/test/lib/saml20.responseSignedMessage.spec.ts +++ b/test/lib/saml20.responseSignedMessage.spec.ts @@ -18,6 +18,7 @@ const inResponseTo = 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'; describe('saml20.responseSignedMessage', function () { it('Should validate saml 2.0 token using thumbprint', async function () { const response = await validate(validResponse, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -32,6 +33,7 @@ describe('saml20.responseSignedMessage', function () { it('Should validate saml 2.0 token using certificate', async function () { const response = await validate(validResponse, { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -71,6 +73,7 @@ describe('saml20.responseSignedMessage', function () { it('Should fail with missing root element', async function () { try { await validate('invalid-assertion', { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -99,6 +102,7 @@ describe('saml20.responseSignedMessage', function () { describe('saml20.validResponseSignedMessageUnsanitized', function () { it('Should validate saml 2.0 token using thumbprint', async function () { const response = await validate(validResponseUnsanitized, { + audience, thumbprint: thumbprint, bypassExpiration: true, inResponseTo: inResponseTo, @@ -113,6 +117,7 @@ describe('saml20.validResponseSignedMessageUnsanitized', function () { it('Should validate saml 2.0 token using certificate', async function () { const response = await validate(validResponseUnsanitized, { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, @@ -152,6 +157,7 @@ describe('saml20.validResponseSignedMessageUnsanitized', function () { it('Should fail with missing root element', async function () { try { await validate('invalid-assertion', { + audience, publicKey: certificate, bypassExpiration: true, inResponseTo: inResponseTo, diff --git a/test/lib/saml20.spec.ts b/test/lib/saml20.spec.ts index c1d5a90d..a62169c3 100644 --- a/test/lib/saml20.spec.ts +++ b/test/lib/saml20.spec.ts @@ -79,53 +79,33 @@ describe('saml20.ts', function () { } }); - it('validateAudience with Suffix ok', async function () { - try { - const value = saml20.validateAudience(assertion1, 'https://saml.boxyhq.com'); - assert.strictEqual(value, true); - } catch (error) { - assert(error); - } + it('validateAudience accepts the exact audience', function () { + const value = saml20.validateAudience( + assertion1, + 'https://saml.boxyhq.com/f46e93394c5a51d36715d95d910872ac7372d4d9' + ); + assert.strictEqual(value, true); }); - it('validateAudience with Suffix not ok with strict check', async function () { - try { - const value = saml20.validateAudience(assertion1, 'https://saml.boxyhq.com', true); - assert.strictEqual(value, false); - } catch (error) { - assert(error); - } + it('validateAudience rejects an expected audience that is only a prefix', function () { + assert.strictEqual(saml20.validateAudience(assertion1, 'https://saml.boxyhq.com'), false); }); - it('validateAudience with Suffix Array ok', async function () { - try { - const value = saml20.validateAudience(assertion1, [...validateOptsArray, 'https://saml.boxyhq.com']); - assert.strictEqual(value, true); - } catch (error) { - assert(error); - } + it('validateAudience accepts the exact audience in a list', function () { + const value = saml20.validateAudience(assertion1, [ + ...validateOptsArray, + 'https://saml.boxyhq.com/f46e93394c5a51d36715d95d910872ac7372d4d9', + ]); + assert.strictEqual(value, true); }); - it('validateAudience with Suffix Array not ok with strict check', async function () { - try { - const value = saml20.validateAudience( - assertion1, - [...validateOptsArray, 'https://saml.boxyhq.com'], - true - ); - assert.strictEqual(value, false); - } catch (error) { - assert(error); - } + it('validateAudience rejects a prefix in a list', function () { + const value = saml20.validateAudience(assertion1, [...validateOptsArray, 'https://saml.boxyhq.com']); + assert.strictEqual(value, false); }); - it('validateAudience with Suffix Array not ok', async function () { - try { - const value = saml20.validateAudience(assertion1, validateOptsArray); - assert.strictEqual(value, false); - } catch (error) { - assert(error); - } + it('validateAudience rejects a list without the audience', function () { + assert.strictEqual(saml20.validateAudience(assertion1, validateOptsArray), false); }); it('validateAudience not ok', function () { @@ -199,39 +179,38 @@ describe('validateAudience with several Audience values', function () { const assertion = { Conditions: { AudienceRestriction: { Audience: [{ _: 'https://other' }, { _: sp }] } }, }; - assert.strictEqual(saml20.validateAudience(assertion, sp, true), true); - assert.strictEqual(saml20.validateAudience(assertion, sp, false), true); + assert.strictEqual(saml20.validateAudience(assertion, sp), true); }); it('accepts several plain-string Audience values', function () { const assertion = { Conditions: { AudienceRestriction: { Audience: ['https://other', sp] } } }; - assert.strictEqual(saml20.validateAudience(assertion, sp, true), true); + assert.strictEqual(saml20.validateAudience(assertion, sp), true); }); it('rejects when no Audience in the AudienceRestriction matches', function () { const assertion = { Conditions: { AudienceRestriction: { Audience: ['https://a', 'https://b'] } } }; - assert.strictEqual(saml20.validateAudience(assertion, sp, true), false); + assert.strictEqual(saml20.validateAudience(assertion, sp), false); }); it('requires every AudienceRestriction to be satisfied', function () { assert.strictEqual( - saml20.validateAudience(withAudiences({ Audience: sp }, { Audience: ['https://x', sp] }), sp, true), + saml20.validateAudience(withAudiences({ Audience: sp }, { Audience: ['https://x', sp] }), sp), true ); assert.strictEqual( - saml20.validateAudience(withAudiences({ Audience: sp }, { Audience: 'https://x' }), sp, true), + saml20.validateAudience(withAudiences({ Audience: sp }, { Audience: 'https://x' }), sp), false ); }); it('matches against any of several expected audiences', function () { const assertion = { Conditions: { AudienceRestriction: { Audience: ['https://a', sp] } } }; - assert.strictEqual(saml20.validateAudience(assertion, ['https://z', sp], true), true); - assert.strictEqual(saml20.validateAudience(assertion, ['https://z'], true), false); + assert.strictEqual(saml20.validateAudience(assertion, ['https://z', sp]), true); + assert.strictEqual(saml20.validateAudience(assertion, ['https://z']), false); }); it('rejects an AudienceRestriction with no Audience', function () { - assert.strictEqual(saml20.validateAudience(withAudiences({}), sp, true), false); + assert.strictEqual(saml20.validateAudience(withAudiences({}), sp), false); }); }); @@ -353,8 +332,8 @@ describe('saml20 time windows, recipients and expiry edges', function () { describe('validateAudience', function () { it('returns false for an expected audience that is not a string', function () { const assertion = { Conditions: { AudienceRestriction: { Audience: 'https://sp' } } }; - assert.strictEqual(saml20.validateAudience(assertion, 42, true), false); - assert.strictEqual(saml20.validateAudience(assertion, { url: 'https://sp' }, false), false); + assert.strictEqual(saml20.validateAudience(assertion, 42), false); + assert.strictEqual(saml20.validateAudience(assertion, { url: 'https://sp' }), false); }); }); }); diff --git a/test/lib/sign.spec.ts b/test/lib/sign.spec.ts index 682ee1f1..1bb6378d 100644 --- a/test/lib/sign.spec.ts +++ b/test/lib/sign.spec.ts @@ -7,8 +7,8 @@ import fs from 'fs'; const validXml = 'me'; -const signingKey = fs.readFileSync('./test/assets/certificates/oktaPrivateKey.pem').toString(); -const publicKey = fs.readFileSync('./test/assets/certificates/oktaPublicKey.crt').toString(); +const signingKey = fs.readFileSync('./test/assets/certificates/testIdpKey.pem').toString(); +const publicKey = fs.readFileSync('./test/assets/certificates/testIdpCert.crt').toString(); describe('sign.ts', function () { it('should sign valid XML', function () { @@ -53,6 +53,8 @@ describe('sign.ts output', function () { it('does not verify after the signed content changes', function () { const signed = sign(validXml, key, cert, assertionXPath); - assert.strictEqual(validateSignature(signed.replace('>me<', '>you<'), cert, null), null); + assert.throws(() => validateSignature(signed.replace('>me<', '>you<'), cert, null), { + message: 'invalid signature: the signature does not verify', + }); }); }); diff --git a/test/lib/validateSignature.spec.ts b/test/lib/validateSignature.spec.ts index 14d9a7c7..b3d0fcfc 100644 --- a/test/lib/validateSignature.spec.ts +++ b/test/lib/validateSignature.spec.ts @@ -1,11 +1,11 @@ -import { certToPEM, validateSignature, DEFAULT_MAX_SIGNATURE_REFERENCES } from '../../lib/validateSignature'; +import { validateSignature, DEFAULT_MAX_SIGNATURE_REFERENCES } from '../../lib/validateSignature'; import xmlbuilder from 'xmlbuilder'; import crypto from 'crypto'; import fs from 'fs'; import { sign } from '../../lib/sign'; import { SignedXml } from 'xml-crypto'; -import { PubKeyInfo, stripCertHeaderAndFooter } from '../../lib/cert'; +import { certToPEM, PubKeyInfo, stripCertHeaderAndFooter } from '../../lib/cert'; import { thumbprint } from '../../lib/utils'; import assert from 'assert'; import { @@ -403,6 +403,21 @@ sT/txBnVJGziyO8DPYdu2fPMER8ajJfl }); }); +describe('validateSignature() failure mode', function () { + const signed = () => signXml(buildAssertion(), ASSERTION_XPATH); + + it('returns the signed XML for a signature that verifies', function () { + assert.match(validateSignature(signed(), testIdpCert, null), /jo@example\.com/); + }); + + it('throws, never returns null, for a signature that does not verify', function () { + const tampered = signed().replace('jo@example.com', 'eve@example.com'); + assert.throws(() => validateSignature(tampered, testIdpCert, null), { + message: 'invalid signature: the signature does not verify', + }); + }); +}); + describe('validateSignature.ts - algorithm allowlists', function () { const sha2Only = { allowedSignatureAlgorithms: [RSA_SHA256], allowedHashAlgorithms: [SHA256] }; @@ -727,7 +742,9 @@ describe('validateSignature.ts - unusual documents', function () { it('does not verify after tampering', function () { const tampered = signedDeep.replace(`${IDP}<`, 'https://evil.example.com<'); assert.notStrictEqual(tampered, signedDeep); - assert.strictEqual(validateSignature(tampered, testIdpCert, null), null); + assert.throws(() => validateSignature(tampered, testIdpCert, null), { + message: 'invalid signature: the signature does not verify', + }); }); }); }); @@ -735,7 +752,7 @@ describe('validateSignature.ts - unusual documents', function () { describe('validateSignature.ts - thumbprint uses the KeyInfo of the signature being verified', function () { const trustedThumbprint = thumbprint(stripCertHeaderAndFooter(testIdpCert)); const otherCert = stripCertHeaderAndFooter( - fs.readFileSync('./test/assets/certificates/oktaPublicKey.crt').toString() + fs.readFileSync('./test/assets/certificates/oneloginPublicKey.crt').toString() ); const signedAssertion = () => signXml(buildAssertion(), ASSERTION_XPATH);