Repository navigation
Add SAML traffic recorder to compare EngineBlock output between commits - #2138
Open
kayjoosten wants to merge 1 commit into
Open
kayjoosten wants to merge 1 commit into
kayjoosten wants to merge 1 commit into
Conversation
# Why is this change needed? Prior to this change, there was no way to see whether a library upgrade changed the SAML messages EngineBlock sends. The Behat suite only asserts selected fields, so a change in a message that no step checks goes unnoticed. The staged migration to simplesamlphp/saml2 v7 rewrites the code that builds and signs messages, so a wire level safety net is needed first. # How does it address the issue? This change adds test-only tooling that is only loaded in the ci and test environments: - a kernel response subscriber that records every SAMLRequest and SAMLResponse that leaves EngineBlock or one of the mocks, for both HTTP-Redirect and HTTP-POST - a normaliser that dumps a message as a prefix independent text tree and masks volatile values (timestamps, identifiers, signature and digest values, time based mock fixtures) while keeping references between messages visible - the engineblock:saml:normalize-recording command to normalise a recording - a Behat context that switches recording on when EB_SAML_RECORD_DIR is set. Behat reaches the application over HTTP, so the setting is handed over through a marker file in /tmp/eb-fixtures Nothing is recorded unless EB_SAML_RECORD_DIR is set. To compare two commits, run the default Behat suite on each with a different directory, normalise both recordings and run diff -r on the results. Two full runs on identical code give an empty diff over 904 messages in 257 scenarios. Changing the Comparison of the Stepup request leaves all 25 Stepup scenarios green, but the recording diff flags 20 of them. # Links #2136
kayjoosten
force-pushed
the
feature/2136-saml2-baseline-and-seams
branch
from
October 22, 2026 07:47
afa8686 to
68ccbca
Compare
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
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.
Summary
Part of #2136 (Phase 0a, the safety net before the saml2 migration).
Adds opt-in, test-only tooling that records the SAML messages EngineBlock and the Behat mocks send during a Behat run, and normalises them so two runs can be compared with
diff.Why
The Behat suite only asserts selected fields of the messages. A change to a message that no step checks goes unnoticed. The migration to
simplesamlphp/saml2v7 rewrites the code that builds and signs messages (about 84 files referenceSAML2\), so a wire level comparison is needed before that work starts.What
SamlTrafficRecorder: kernel response subscriber that records every SAMLRequest and SAMLResponse leaving the application (HTTP-Redirect and HTTP-POST). It covers EngineBlock and the mocks.SamlXmlNormalizer: dumps a message as a prefix independent text tree. Volatile values (timestamps, identifiers, signature and digest values, time based mock fixtures) are masked, and references between messages stay visible asID-n.engineblock:saml:normalize-recording: normalises a recording directory.SamlRecordingContext: switches recording on whenEB_SAML_RECORD_DIRis set. Behat talks to Apache over HTTP, so the setting reaches the application through a marker file in/tmp/eb-fixtures, which is removed after the suite.Everything is only registered in the
ciandtestenvironments (config/bundles.php,config/services_ci.yaml). Nothing is recorded unlessEB_SAML_RECORD_DIRis set, and no production code is touched.How to use
Inside the container as
www-data, reset thecischema before each run:Verification
Comparisonof the Stepup AuthnRequest fromminimumtoexactleaves all 25 Stepup scenarios green, but the recording diff flags 20 of them (@Comparison: minimumdisappears). The change was reverted.EB_SAML_RECORD_DIRunset the suite passes 300/300 and nothing is written.lint:twig. PHPUnit: eb4 244, unit 1064, functional 123, integration 104. Behat default suite 300/300.Lifetime
This is a temporary migration aid, not a permanent test suite. It is used on the v5, v6 and v7 upgrade commits and is meant to be removed once the v7 commit has landed. The permanent safety net (unit tests on the EngineBlock owned seams and a corpus of inbound IdP responses) follows in separate PRs.
Known limits: it only sees what Behat exercises, it does not show whether the library becomes stricter about inbound IdP responses, and signature values are masked.
Not in this PR
The EngineBlock owned seams (Phase 0b) and the staged v5, v6 and v7 upgrade commits. Those follow separately. This PR is independent of #2135.