Repository navigation
fix: okta sibling case - #151
Merged
Merged
Conversation
decryptKeyInfo located elements with XPath expressions that were built by
string concatenation and scoped by a leading "//". Both properties made the
lookups behave differently from how they read.
"//" is descendant-or-self over the whole document regardless of the context
node passed to select(), so lookups that appear scoped were document-wide:
- the wrapped key was taken from the first CipherValue anywhere in the
document rather than from the EncryptedKey being processed
- DigestMethod was resolved by an absolute path, so with several
EncryptedKey elements it could return a digest declared by one key while
the ciphertext came from another
- the absolute DigestMethod path also matched nothing for documents using
EncryptedData/KeyInfo/RetrievalMethod, where EncryptedKey sits outside
KeyInfo, silently defaulting oaepHash to sha1
Predicates were also assembled by concatenating the RetrievalMethod URI into
the expression text, so an Id containing a quote changed what the expression
meant instead of being compared as a value.
lib/dom-select.js walks the DOM explicitly. Document values are compared,
never concatenated into anything that gets parsed, and scope is a node
enforced by the walk itself. EncryptionMethod and CipherValue are now read
from one resolved EncryptedKey, so they always describe the same key.
Two input-validation changes come with this:
- a RetrievalMethod URI must be a same-document fragment reference.
substring(1) removed the first character whatever it was, so "/ek1" and
"Xek1" both resolved to Id "ek1".
- a duplicated Id is reported rather than resolved to its first match.
Duplicate Ids are invalid XML and resolving them makes the result depend
on document order.
xpath moves to devDependencies; the runtime no longer parses expressions.
Verified with a packed tarball installed via --omit=dev, where
require('xpath') throws MODULE_NOT_FOUND and both fixtures still decrypt.
65 passing (44 before). Each property is pinned by a test that also asserts
the previous XPath behaviour for contrast, and 11 mutations of the traversal
and the guards were each confirmed to fail a named test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tures with mgf1=sha1
adamjmcgrath
approved these changes
Oct 13, 2026
adamjmcgrath
left a comment
adamjmcgrath
left a comment
Contributor
There was a problem hiding this comment.
tested with okta and samlmock - lgtm
jakelacey2012
approved these changes
Oct 15, 2026
samiksharana-okta
approved these changes
Oct 15, 2026
Contributor
|
🎉 This PR is included in version 6.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
decryptKeyInfonow resolves aRetrievalMethodreference to anEncryptedKeythat sits next toEncryptedData. This is the layout Okta uses, where theEncryptedKeyis a sibling ofEncryptedDataunderEncryptedAssertion. It is valid XML Encryption and SAML 2.0. Previously the lookup only searched inside theEncryptedDataelement, so callers that pass that element alone could not find the key.Changes
RetrievalMethod#idis now resolved within the parent ofEncryptedData(the enclosingEncryptedAssertion) instead of only theEncryptedDatasubtree. The same-document#idrequirement and the duplicate-Id rejection are unchanged.EncryptedData:decryptanddecryptKeyInforesolve the firstEncryptedDataonce. The key, algorithm, and ciphertext are all read from that block, so they can no longer come from differentEncryptedDataorEncryptedKeyelements in the same document.KeyInfo,EncryptedKey,RetrievalMethod,EncryptionMethod, andCipherData/CipherValueare read as direct children of the resolved element. AnEncryptedKeynested deeper than a direct child ofKeyInfois no longer found.Testing
EncryptedDatablocks.test/test-okta-sibling-key.xml) checks that the key is found. It uses dummy identifiers and ciphertext.EncryptedDataelement.Checklist