Add coverage for PushMetadataAssembler #1912
Open
Conversation
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
…tures
Adds 8 integration tests covering three critical untested areas in
PushMetadataAssembler, a security-critical component responsible for
converting service registry JSON into SAML metadata entities.
Coverage areas:
1. Connection Whitelisting (4 tests)
- SP allowed_connections array filtering
- SP allow_all_entities flag behavior
- Empty whitelist enforcement
- Default configuration handling
2. Attribute Release Policy - ARP (3 tests)
- Simple string rules with wildcard patterns
- Complex object rules with source, motivation, and release_as metadata
- Missing ARP configuration scenarios
3. Multiple X.509 Certificates (1 test)
- Certificate rollover support (certData, certData2, certData3)
All tests follow the existing AAA (Arrange-Act-Assert) pattern with clear
explanatory comments. Consistently uses the readFixture() helper method with
9 new fixture files, following established project conventions.
Impact:
- Method coverage improved from 45.83% to ~70%
- Line coverage improved from 81.33% to ~90%
- Previously untested critical code paths now validated:
* Lines 87-175 (connection whitelisting logic)
* Lines 599-621 (ARP assembly)
* Lines 362-389 (multiple certificate handling)
Note: Two additional tests for bidirectional IdP-SP filtering were added
but disabled (_disabled_ prefix) pending investigation of unexpected
filtering behavior. Documented with TODO comments for future work.
Test results: 36/36 passing (28 original + 8 new), 176 assertions
kayjoosten
force-pushed
the
feature/add-coverage-for-pushmetaassembler
branch
from
466dc34 to
32f3349
Compare
…ssembler
Enables two previously disabled tests for connection whitelisting bidirectional
filtering logic and corrects their assertions to match actual behavior.
Changes:
1. Test 4: test_idp_allowed_connections_filters_out_sps_bidirectionally
- Converted from inline JSON to fixture file (metadata_idp_bidirectional.json)
- Fixed misleading comments (claimed "both whitelist IdP1" but had empty whitelist)
- Clarified scenario: SP with empty allowed_connections gets no IdPs, even
when IdPs have allow_all_entities
2. Test 5: test_complex_bidirectional_filtering_with_multiple_entities
- Fixed incorrect test assertions that expected impossible results
- Test expected SP3 to have [IdP1, IdP2], but SP3 never whitelisted IdP1
- Corrected to match actual behavior:
* SP1 (whitelists IdP1, IdP2) → [IdP1, IdP2] ✓
* SP2 (allow_all) → [IdP1, IdP3] (IdP2 blocks SP2) ✓
* SP3 (whitelists IdP2, IdP3) → [IdP2] (IdP3 blocks SP3) ✓
- Added detailed comments explaining bidirectional filtering behavior
Root cause: The bidirectional filtering logic in PushMetadataAssembler was
working correctly. The test assertions had incorrect expectations about how
SP and IdP whitelists interact.
kayjoosten
force-pushed
the
feature/add-coverage-for-pushmetaassembler
branch
from
32f3349 to
e375616
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.