REF-15: Fix transposed default audit attribute sources for SESSION_ID and USER - #89
Open
thomasnymand wants to merge 1 commit into
Open
REF-15: Fix transposed default audit attribute sources for SESSION_ID and USER#89thomasnymand wants to merge 1 commit into
thomasnymand wants to merge 1 commit into
Conversation
… and USER The default lookup specs for the SessionId and ServiceProviderUserId audit attributes were swapped in Configuration.Builder.build(): the SessionId attribute defaulted to "request:remoteUser" and the ServiceProviderUserId attribute to "request:sessionId". Because AuditRequestUtil resolves these specs literally (request:sessionId -> session id, request:remoteUser -> remote user), the audit log's SESSION_ID column was populated with the remote user and the USER column with the session id. Swap the two defaults so each column is sourced correctly, which also matches the documented defaults. Add ConfigurationTest asserting the default audit attribute values and register the previously untested dk.gov.oio.saml.config package in TestSuite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
The default lookup specs for the
SessionIdandServiceProviderUserIdaudit attributes were swapped inConfiguration.Builder.build():auditRequestAttributeSessionIddefaulted torequest:remoteUserauditRequestAttributeServiceProviderUserIddefaulted torequest:sessionIdAuditRequestUtilresolves these specs literally (request:sessionId→ session id,request:remoteUser→ remote user), so with the default configuration the audit log'sSESSION_IDcolumn was populated with the remote user and theUSERcolumn with the session id. Nothing downstream compensated — the emitted audit records were genuinely wrong.Fix
Tests
ConfigurationTestasserting the default audit attribute values (fails against the old transposed defaults, passes with the fix).dk.gov.oio.saml.configpackage inTestSuiteso the new test actually runs.Discovered during the REF-14 documentation review (#88).
Note: two unrelated tests (
OIOBPPUtilTest,CRLCheckerTest) fail in the local environment onmasteras well — JDK 26 JAXB and live-network OCSP respectively — and are not affected by this change.🤖 Generated with Claude Code