From 622a91a152ffe37c64a153b0e3a31f6cc07e3ab7 Mon Sep 17 00:00:00 2001 From: Guy Needham Date: Thu, 15 Jan 2026 15:18:40 +0000 Subject: [PATCH 1/3] add verification to NotOnOrAfter date --- lib/saml2.coffee | 17 ++++++++++++ test/saml2.coffee | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/lib/saml2.coffee b/lib/saml2.coffee index 64c9cd5..4288df2 100644 --- a/lib/saml2.coffee +++ b/lib/saml2.coffee @@ -488,6 +488,23 @@ parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_ ) if !validAudience? return cb_wf new SAMLError('SAML Response is not valid for this audience') + + # Validate SubjectConfirmationData NotOnOrAfter + if ignore_timing != true + subject = decrypted_assertion.getElementsByTagNameNS(XMLNS.SAML, 'Subject')[0] + if subject? + subject_confirmation = subject.getElementsByTagNameNS(XMLNS.SAML, 'SubjectConfirmation')[0] + if subject_confirmation? + subject_confirmation_data = subject_confirmation.getElementsByTagNameNS(XMLNS.SAML, 'SubjectConfirmationData')[0] + if subject_confirmation_data? + not_on_or_after = subject_confirmation_data.getAttribute('NotOnOrAfter') + if not_on_or_after? + not_on_or_after_date = Date.parse(not_on_or_after) + if isNaN(not_on_or_after_date) + return cb_wf new SAMLError('SAML Subject has invalid NotOnOrAfter date', {NotOnOrAfter: not_on_or_after}) + if not_on_or_after_date <= Date.now() + return cb_wf new SAMLError('SAML Subject is no longer valid', {NotOnOrAfter: not_on_or_after}) + return cb_wf null, decrypted_assertion (validated_assertion, cb_wf) -> # Populate attributes diff --git a/test/saml2.coffee b/test/saml2.coffee index 6220603..fef6847 100644 --- a/test/saml2.coffee +++ b/test/saml2.coffee @@ -778,6 +778,9 @@ describe 'saml2', -> .replace 'NotBefore="2054-03-12T21:35:05.387Z"', # mimicking an IdP with a clock 3 seconds ahead of ours "NotBefore=\"#{new Date(Date.now()+3000).toISOString()}\"" + .replace 'NotOnOrAfter="2014-03-12T21:40:05.392Z"', + # also update SubjectConfirmationData NotOnOrAfter to be in the future + "NotOnOrAfter=\"#{new Date(Date.now()+60000).toISOString()}\"" saml_response_base64 = Buffer.from(saml_response, 'utf8').toString('base64') request_options = require_session_index: false @@ -848,6 +851,65 @@ describe 'saml2', -> assert (/SAML Response is no longer valid/.test(err.message)), "Unexpected error message:" + err.message done() + it 'rejects an assertion with a SubjectConfirmationData NotOnOrAfter in the past', (done) -> + sp_options = + entity_id: 'https://sp.example.com/metadata.xml' + private_key: get_test_file('test2.pem') + alt_private_keys: get_test_file('test.pem') + certificate: get_test_file('test2.crt') + alt_certs: get_test_file('test.crt') + assert_endpoint: 'https://sp.example.com/assert' + idp_options = + sso_login_url: 'https://idp.example.com/login' + sso_logout_url: 'https://idp.example.com/logout' + certificates: [ get_test_file('test.crt'), get_test_file('test2.crt') ] + request_options = + require_session_index: false + ignore_signature: true + allow_unencrypted_assertion: true + request_body: + SAMLResponse: get_test_file("response_audience_no_timing.xml") + + sp = new saml2.ServiceProvider sp_options + idp = new saml2.IdentityProvider idp_options + + sp.post_assert idp, request_options, (err, response) -> + assert (err instanceof Error), "Did not get expected error." + assert (/SAML Subject is no longer valid/.test(err.message)), "Unexpected error message:" + err.message + done() + + it 'rejects an assertion with an invalid SubjectConfirmationData NotOnOrAfter date', (done) -> + sp_options = + entity_id: 'https://sp.example.com/metadata.xml' + private_key: get_test_file('test2.pem') + alt_private_keys: get_test_file('test.pem') + certificate: get_test_file('test2.crt') + alt_certs: get_test_file('test.crt') + assert_endpoint: 'https://sp.example.com/assert' + idp_options = + sso_login_url: 'https://idp.example.com/login' + sso_logout_url: 'https://idp.example.com/logout' + certificates: [ get_test_file('test.crt'), get_test_file('test2.crt') ] + + # Modify test file to have an invalid date (file is base64 encoded, decode first) + saml_response_decoded = Buffer.from(get_test_file("response_audience_no_timing.xml"), 'base64').toString('utf8') + .replace 'NotOnOrAfter="2014-03-12T21:40:05.392Z"', 'NotOnOrAfter="not-a-valid-date"' + saml_response_base64 = Buffer.from(saml_response_decoded, 'utf8').toString('base64') + request_options = + require_session_index: false + ignore_signature: true + allow_unencrypted_assertion: true + request_body: + SAMLResponse: saml_response_base64 + + sp = new saml2.ServiceProvider sp_options + idp = new saml2.IdentityProvider idp_options + + sp.post_assert idp, request_options, (err, response) -> + assert (err instanceof Error), "Did not get expected error." + assert (/SAML Subject has invalid NotOnOrAfter date/.test(err.message)), "Unexpected error message:" + err.message + done() + context 'when response contains AudienceRestriction', -> sp_options = (properties = {}) -> _.extend @@ -866,6 +928,7 @@ describe 'saml2', -> _.extend require_session_index: false ignore_signature: true + ignore_timing: true allow_unencrypted_assertion: true request_body: SAMLResponse: get_test_file("response_audience_no_timing.xml") @@ -940,6 +1003,7 @@ describe 'saml2', -> request_options = require_session_index: false ignore_signature: true + ignore_timing: true allow_unencrypted_assertion: true request_body: SAMLResponse: get_test_file("response_no_audience_no_timing.xml") @@ -1366,6 +1430,7 @@ describe 'saml2', -> request_options = ignore_signature: true + ignore_timing: true allow_unencrypted_assertion: true request_body: SAMLResponse: get_test_file("response_without_issuer.xml") @@ -1386,6 +1451,7 @@ describe 'saml2', -> request_options = ignore_signature: true + ignore_timing: true allow_unencrypted_assertion: true request_body: SAMLResponse: get_test_file("response_with_issuer.xml") @@ -1406,6 +1472,7 @@ describe 'saml2', -> request_options = ignore_signature: true + ignore_timing: true allow_unencrypted_assertion: true request_body: SAMLResponse: get_test_file("response_with_issuer.xml") From aead004b6f5c90567303f5eef866b6bb1ac73fe9 Mon Sep 17 00:00:00 2001 From: Guy Needham Date: Mon, 2 Feb 2026 17:11:30 +0000 Subject: [PATCH 2/3] add TypeScript types to package --- index.d.ts | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ package.json | 1 + 2 files changed, 74 insertions(+) create mode 100644 index.d.ts diff --git a/index.d.ts b/index.d.ts new file mode 100644 index 0000000..23d93ae --- /dev/null +++ b/index.d.ts @@ -0,0 +1,73 @@ +export type ServiceProviderOptions = { + entity_id: string; + private_key: string; + certificate: string; + assert_endpoint: string; +}; + +export type IdentityProviderOptions = { + sso_login_url: string; + sso_logout_url?: string; + certificates?: string[]; + force_authn: boolean; + sign_get_request: boolean; + allow_unencrypted_assertion: boolean; +}; + +export type SAMLUser = { + name_id: string; + session_index: string; + attributes?: Record; +}; + +export type SAMLResponse = { + user: SAMLUser; +}; + +export type SAMLLogoutRequest = { + type: "logout_request"; + name_id: string; + session_index: string; + response_header: { id: string }; + RelayState?: string; +}; + +export type SAMLLogoutResponse = { + type: "logout_response"; +}; + +export type SAMLRequestResponse = SAMLLogoutRequest | SAMLLogoutResponse; + +export class ServiceProvider { + constructor(options: ServiceProviderOptions); + create_metadata(): string; + create_login_request_url( + idp: IdentityProvider, + options: Record, + callback: (err: Error | null, login_url: string, request_id: string) => void, + ): void; + post_assert( + idp: IdentityProvider, + options: { request_body: Record }, + callback: (err: Error | null, saml_response: SAMLResponse) => void, + ): void; + redirect_assert( + idp: IdentityProvider, + options: { request_body: Record }, + callback: (err: Error | null, saml_reqrep: SAMLRequestResponse) => void, + ): void; + create_logout_request_url( + idp: IdentityProvider, + options: { name_id?: string; session_index?: string }, + callback: (err: Error | null, logout_url: string) => void, + ): void; + create_logout_response_url( + idp: IdentityProvider, + options: { in_response_to: string; relay_state?: string }, + callback: (err: Error | null, response_url: string) => void, + ): void; +} + +export class IdentityProvider { + constructor(options: IdentityProviderOptions); +} diff --git a/package.json b/package.json index 9d1c113..92347ae 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "author": "Clever", "license": "Apache-2.0", "main": "index.js", + "types": "index.d.ts", "engines": { "node": ">=14" }, From 4b38008c7a2c462d6bfccbb7b997e14f2505137b Mon Sep 17 00:00:00 2001 From: Guy Needham Date: Mon, 16 Mar 2026 10:24:44 +0000 Subject: [PATCH 3/3] fix getAttribute returning empty string for missing NotOnOrAfter In @xmldom/xmldom 0.8.x, getAttribute returns "" for non-existent attributes, not null. The CoffeeScript existential check only guards against null/undefined, so an empty string passed through causing Date.parse("") to return NaN and falsely reject valid SAML responses where NotOnOrAfter is optional per SAML spec. Use hasAttribute to properly check if the attribute exists before getting its value. Added tests for: - Missing NotOnOrAfter attribute (should be accepted) - Empty NotOnOrAfter attribute (should be rejected) - Malformed NotOnOrAfter attribute (should be rejected) Co-Authored-By: Claude Opus 4.5 --- lib/saml2.coffee | 13 +++---- test/saml2.coffee | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 7 deletions(-) diff --git a/lib/saml2.coffee b/lib/saml2.coffee index 4288df2..6c73b10 100644 --- a/lib/saml2.coffee +++ b/lib/saml2.coffee @@ -496,14 +496,13 @@ parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_ subject_confirmation = subject.getElementsByTagNameNS(XMLNS.SAML, 'SubjectConfirmation')[0] if subject_confirmation? subject_confirmation_data = subject_confirmation.getElementsByTagNameNS(XMLNS.SAML, 'SubjectConfirmationData')[0] - if subject_confirmation_data? + if subject_confirmation_data? and subject_confirmation_data.hasAttribute('NotOnOrAfter') not_on_or_after = subject_confirmation_data.getAttribute('NotOnOrAfter') - if not_on_or_after? - not_on_or_after_date = Date.parse(not_on_or_after) - if isNaN(not_on_or_after_date) - return cb_wf new SAMLError('SAML Subject has invalid NotOnOrAfter date', {NotOnOrAfter: not_on_or_after}) - if not_on_or_after_date <= Date.now() - return cb_wf new SAMLError('SAML Subject is no longer valid', {NotOnOrAfter: not_on_or_after}) + not_on_or_after_date = Date.parse(not_on_or_after) + if isNaN(not_on_or_after_date) + return cb_wf new SAMLError('SAML Subject has invalid NotOnOrAfter date', {NotOnOrAfter: not_on_or_after}) + if not_on_or_after_date <= Date.now() + return cb_wf new SAMLError('SAML Subject is no longer valid', {NotOnOrAfter: not_on_or_after}) return cb_wf null, decrypted_assertion (validated_assertion, cb_wf) -> diff --git a/test/saml2.coffee b/test/saml2.coffee index fef6847..5701fc7 100644 --- a/test/saml2.coffee +++ b/test/saml2.coffee @@ -910,6 +910,101 @@ describe 'saml2', -> assert (/SAML Subject has invalid NotOnOrAfter date/.test(err.message)), "Unexpected error message:" + err.message done() + it 'accepts an assertion with no SubjectConfirmationData NotOnOrAfter attribute', (done) -> + sp_options = + entity_id: 'https://sp.example.com/metadata.xml' + private_key: get_test_file('test2.pem') + alt_private_keys: get_test_file('test.pem') + certificate: get_test_file('test2.crt') + alt_certs: get_test_file('test.crt') + assert_endpoint: 'https://sp.example.com/assert' + idp_options = + sso_login_url: 'https://idp.example.com/login' + sso_logout_url: 'https://idp.example.com/logout' + certificates: [ get_test_file('test.crt'), get_test_file('test2.crt') ] + + # Modify test file to remove NotOnOrAfter from SubjectConfirmationData (NotOnOrAfter is optional per SAML spec) + saml_response_decoded = Buffer.from(get_test_file("response_audience_no_timing.xml"), 'base64').toString('utf8') + .replace /NotOnOrAfter="2014-03-12T21:40:05.392Z" /, '' + saml_response_base64 = Buffer.from(saml_response_decoded, 'utf8').toString('base64') + request_options = + require_session_index: false + ignore_signature: true + allow_unencrypted_assertion: true + request_body: + SAMLResponse: saml_response_base64 + + sp = new saml2.ServiceProvider sp_options + idp = new saml2.IdentityProvider idp_options + + sp.post_assert idp, request_options, (err, response) -> + assert !err?, "Got unexpected error: #{err}" + done() + + it 'rejects an assertion with an empty SubjectConfirmationData NotOnOrAfter attribute', (done) -> + sp_options = + entity_id: 'https://sp.example.com/metadata.xml' + private_key: get_test_file('test2.pem') + alt_private_keys: get_test_file('test.pem') + certificate: get_test_file('test2.crt') + alt_certs: get_test_file('test.crt') + assert_endpoint: 'https://sp.example.com/assert' + idp_options = + sso_login_url: 'https://idp.example.com/login' + sso_logout_url: 'https://idp.example.com/logout' + certificates: [ get_test_file('test.crt'), get_test_file('test2.crt') ] + + # Modify test file to have an empty NotOnOrAfter attribute + saml_response_decoded = Buffer.from(get_test_file("response_audience_no_timing.xml"), 'base64').toString('utf8') + .replace 'NotOnOrAfter="2014-03-12T21:40:05.392Z"', 'NotOnOrAfter=""' + saml_response_base64 = Buffer.from(saml_response_decoded, 'utf8').toString('base64') + request_options = + require_session_index: false + ignore_signature: true + allow_unencrypted_assertion: true + request_body: + SAMLResponse: saml_response_base64 + + sp = new saml2.ServiceProvider sp_options + idp = new saml2.IdentityProvider idp_options + + sp.post_assert idp, request_options, (err, response) -> + assert (err instanceof Error), "Did not get expected error." + assert (/SAML Subject has invalid NotOnOrAfter date/.test(err.message)), "Unexpected error message:" + err.message + done() + + it 'rejects an assertion with a malformed SubjectConfirmationData NotOnOrAfter attribute', (done) -> + sp_options = + entity_id: 'https://sp.example.com/metadata.xml' + private_key: get_test_file('test2.pem') + alt_private_keys: get_test_file('test.pem') + certificate: get_test_file('test2.crt') + alt_certs: get_test_file('test.crt') + assert_endpoint: 'https://sp.example.com/assert' + idp_options = + sso_login_url: 'https://idp.example.com/login' + sso_logout_url: 'https://idp.example.com/logout' + certificates: [ get_test_file('test.crt'), get_test_file('test2.crt') ] + + # Modify test file to have a malformed NotOnOrAfter attribute + saml_response_decoded = Buffer.from(get_test_file("response_audience_no_timing.xml"), 'base64').toString('utf8') + .replace 'NotOnOrAfter="2014-03-12T21:40:05.392Z"', 'NotOnOrAfter="2014-99-99T99:99:99.000Z"' + saml_response_base64 = Buffer.from(saml_response_decoded, 'utf8').toString('base64') + request_options = + require_session_index: false + ignore_signature: true + allow_unencrypted_assertion: true + request_body: + SAMLResponse: saml_response_base64 + + sp = new saml2.ServiceProvider sp_options + idp = new saml2.IdentityProvider idp_options + + sp.post_assert idp, request_options, (err, response) -> + assert (err instanceof Error), "Did not get expected error." + assert (/SAML Subject has invalid NotOnOrAfter date/.test(err.message)), "Unexpected error message:" + err.message + done() + context 'when response contains AudienceRestriction', -> sp_options = (properties = {}) -> _.extend