From ff2f57d2344fbefc87e75d8b9d893843afcacfce Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Mon, 6 Jul 2026 15:26:57 +0200 Subject: [PATCH] REF-15: Fix transposed default audit attribute sources for SESSION_ID 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) --- .../dk/gov/oio/saml/config/Configuration.java | 4 +-- .../test/java/dk/gov/oio/saml/TestSuite.java | 1 + .../oio/saml/config/ConfigurationTest.java | 36 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java b/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java index 928bd2d..8128108 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java @@ -515,8 +515,8 @@ public Configuration build() throws InternalException { configuration.auditLoggerClassName = StringUtil.defaultIfEmpty(this.auditLoggerClassName, "dk.gov.oio.saml.audit.Slf4JAuditLogger"); configuration.auditRequestAttributeIP = StringUtil.defaultIfEmpty(this.auditRequestAttributeIP, "request:remoteAddr"); configuration.auditRequestAttributePort = StringUtil.defaultIfEmpty(this.auditRequestAttributePort, "request:remotePort"); - configuration.auditRequestAttributeSessionId = StringUtil.defaultIfEmpty(this.auditRequestAttributeSessionId, "request:remoteUser"); - configuration.auditRequestAttributeServiceProviderUserId = StringUtil.defaultIfEmpty(this.auditRequestAttributeServiceProviderUserId, "request:sessionId"); + configuration.auditRequestAttributeSessionId = StringUtil.defaultIfEmpty(this.auditRequestAttributeSessionId, "request:sessionId"); + configuration.auditRequestAttributeServiceProviderUserId = StringUtil.defaultIfEmpty(this.auditRequestAttributeServiceProviderUserId, "request:remoteUser"); configuration.sessionHandlerFactoryClassName = StringUtil.defaultIfEmpty(this.sessionHandlerFactoryClassName, null); configuration.sessionHandlerJndiName = StringUtil.defaultIfEmpty(this.sessionHandlerJndiName, null); configuration.sessionHandlerJdbcUrl = StringUtil.defaultIfEmpty(this.sessionHandlerJdbcUrl, null); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/TestSuite.java b/oiosaml/src/test/java/dk/gov/oio/saml/TestSuite.java index 2405623..5d475c2 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/TestSuite.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/TestSuite.java @@ -6,6 +6,7 @@ @RunWith(JUnitPlatform.class) @SelectPackages( { + "dk.gov.oio.saml.config", "dk.gov.oio.saml.filter", "dk.gov.oio.saml.oiobpp", "dk.gov.oio.saml.service", diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java new file mode 100644 index 0000000..283a651 --- /dev/null +++ b/oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java @@ -0,0 +1,36 @@ +package dk.gov.oio.saml.config; + +import dk.gov.oio.saml.util.InternalException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class ConfigurationTest { + + private Configuration minimalConfiguration() throws InternalException { + // Only the mandatory fields are supplied; every optional value falls back to its default. + return new Configuration.Builder() + .setSpEntityID("https://sp.example.com") + .setBaseUrl("https://sp.example.com") + .setIdpEntityID("https://idp.example.com") + .setIdpMetadataUrl("https://idp.example.com/metadata") + .setKeystoreLocation("keystore.p12") + .setKeystorePassword("password") + .setKeyAlias("alias") + .build(); + } + + @DisplayName("Default audit request attributes map to the matching request value (REF-15, issue #76 sibling)") + @Test + void testDefaultAuditRequestAttributes() throws InternalException { + Configuration configuration = minimalConfiguration(); + + // The SessionId audit field must default to the session id and the ServiceProviderUserId + // audit field to the remote user - these two defaults were previously transposed, so the + // SESSION_ID column logged the user and the USER column logged the session id. + Assertions.assertEquals("request:sessionId", configuration.getAuditRequestAttributeSessionId()); + Assertions.assertEquals("request:remoteUser", configuration.getAuditRequestAttributeServiceProviderUserId()); + Assertions.assertEquals("request:remoteAddr", configuration.getAuditRequestAttributeIP()); + Assertions.assertEquals("request:remotePort", configuration.getAuditRequestAttributePort()); + } +}