From 7253e773ecf6a0501f632d470c7126b8b66ac020 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 13 Aug 2026 17:06:17 +0200 Subject: [PATCH 1/5] REF-33: Read IdP metadata from a deployed file and accept every signing certificate in it The SP could fetch IdP metadata over HTTP, and did so on a refresh timer, which made the IdP signing certificate replaceable at runtime by whoever answered the metadata URL. NemLog-in does not sign its metadata, so there is nothing in the document itself to verify: trust can only come from the deployment of the file. oiosaml.servlet.trust.selfsigned.certs made this worse by turning off both certificate and hostname validation for that fetch. HTTPMetadataResolver and the HTTP client are removed, oiosaml.servlet.idp .metadata.file is now mandatory, and the resolver requires metadata to be within its validUntil. The file is still re-read while the SP runs, so publishing new keys remains a matter of replacing it. DispatcherServlet refuses to start when oiosaml.servlet.idp.metadata.url or trust.selfsigned.certs is set, naming the replacement, rather than starting with a trust model the deployment does not expect. Without an auto refreshing resolver the SP has to ride a signing key rotation on the metadata it was given, and an IdP publishes both the outgoing and the incoming certificate while rotating. getValidX509Certificate returned only the first one, so half of such a rotation would have failed validation. It is replaced by getValidX509Certificates, and assertion signatures are accepted when they validate against any of the published certificates. Tests deploy metadata as a temporary file rather than serving it from MockServer, and a new test signs an assertion with a certificate that is not first in metadata. --- README.md | 6 +- demo/src/main/resources/oiosaml.properties | 8 +-- .../dk/gov/oio/saml/config/Configuration.java | 29 +------- .../dk/gov/oio/saml/model/IdPMetadata.java | 70 +++++++------------ .../oio/saml/service/IdPMetadataService.java | 6 +- .../AssertionValidationService.java | 23 ++++-- .../oio/saml/servlet/DispatcherServlet.java | 33 +++++++-- .../java/dk/gov/oio/saml/util/Constants.java | 6 +- .../gov/oio/saml/audit/AuditServiceTest.java | 2 +- .../oio/saml/config/ConfigurationTest.java | 2 +- .../saml/filter/AuthenticatedFilterTest.java | 2 +- .../gov/oio/saml/model/IdPMetadataTest.java | 2 +- .../saml/service/AuthnRequestServiceTest.java | 2 +- .../gov/oio/saml/service/BaseServiceTest.java | 2 +- .../saml/service/CredentialServiceTest.java | 2 +- .../saml/service/IdpMetadataServiceTest.java | 31 ++------ .../oio/saml/service/OIOSAML3ServiceTest.java | 4 +- .../AssertionValidationServiceTest.java | 44 ++++++++++++ .../session/SessionDestroyListenerTest.java | 2 +- .../java/dk/gov/oio/saml/util/IdpUtil.java | 8 +++ .../dk/gov/oio/saml/util/TestConstants.java | 20 ++++++ 21 files changed, 170 insertions(+), 134 deletions(-) diff --git a/README.md b/README.md index b270e5f..77ab9e4 100644 --- a/README.md +++ b/README.md @@ -163,8 +163,7 @@ This chapter describes all the configuration parameters and their default values | `oiosaml.servlet.keystore.password` | Yes | | The password to the PKCS#12 keystore given above. | | `oiosaml.servlet.keystore.alias` | Yes | | The alias of the key entry in the PKCS#12 keystore given above. | | `oiosaml.servlet.idp.entityid` | Yes | | The EntityID of the SAML Identity Provider that is used for login. | -| `oiosaml.servlet.idp.metadata.file` | Partially | | A FILE reference to the SAML Identity Provider metadata. The file must be located on the classpath of the application. Note that either a FILE or URL reference is required. | -| `oiosaml.servlet.idp.metadata.url` | Partially | | A URL reference to the SAML Identity Provider metadata. Note that either a FILE or URL reference is required. | +| `oiosaml.servlet.idp.metadata.file` | Yes | | A FILE reference to the SAML Identity Provider metadata. The file must be located on the classpath of the application. Download the metadata from the Identity Provider and deploy it with the application: NemLog-in does not sign its metadata, so trust in it comes from deploying the file, not from the transport it was fetched over. | | `oiosaml.servlet.configurationfile` | No | | A FILE reference to a configuration file. If supplied, the `DispatcherServlet` will read its configuration from that file instead of the `ServletConfig` section. See [DispatcherServlet configuration from file](#dispatcherservlet-configuration-from-file). | | `oiosaml.servlet.profile.validation.enabled` | No | `true` | By default, the framework performs OIO SAML 3.0 profile validation. If this is not needed, turn off this setting by setting the value to `false`. | | `oiosaml.servlet.profile.validation.assurancelevel.allowed` | No | `false` | The NemLog-in IdP cannot for all authentication provide a NSIS LoA. Therefore the service provider can decide to accept the AssuranceLevel which the NemLog-in IdP provides instead. If this is not acceptable, turn off this setting by omitting it or setting the value to `false`. | @@ -180,7 +179,6 @@ This chapter describes all the configuration parameters and their default values | `oiosaml.servlet.secondary.page.error` | No | | The framework has a built-in error page, which is shown in case of SAML related errors. Set this value to redirect the user to another webpage in case of errors. See [Additional configuration](#additional-configuration) for details on getting error information. | | `oiosaml.servlet.secondary.page.logout` | No | | The framework redirects the user to the context root of the web application after a successful logout. Set this value to redirect the user to another webpage instead of the context root. | | `oiosaml.servlet.secondary.page.login` | No | | When the login process completes, the framework attempts to redirect the user to the web-resource they tried to access before login started. If this fails, the framework redirects to the page specified by the value. If no value is specified, the context root of the application is used. | -| `oiosaml.servlet.trust.selfsigned.certs` | No | `false` | By default, the framework performs certificate validation when accessing HTTPS-protected resources like SAML metadata. Set this value to `true` to disable certificate validation. | | `oiosaml.servlet.revocation.crl.check.enabled` | No | `true` | By default, the framework performs revocation checking using OCSP and CRL. Set this value to `false` to disable CRL revocation checking. | | `oiosaml.servlet.revocation.ocsp.check.enabled` | No | `true` | By default, the framework performs revocation checking using OCSP and CRL. Set this value to `false` to disable OCSP revocation checking. | | `oiosaml.servlet.routing.path.prefix` | No | `saml` | Routing configuration: servlet path prefix for the OIO dispatch servlet. Change this to change where the OIOSAML 3 endpoint is mounted in the application context (`oiosaml.servlet.baseurl`). Example: with the default, the logout action is hit at `/{prefix}/{suffix.logout}` = `/saml/logout`. | @@ -217,7 +215,7 @@ If the `oiosaml.servlet.configurationfile` setting is supplied to the `Dispatche ```properties oiosaml.servlet.idp.entityid=https://saml.test-nemlog-in.dk/ -oiosaml.servlet.idp.metadata.url=https://test-nemlog-in.dk/Testportal/Test-nemlog-in-2.xml +oiosaml.servlet.idp.metadata.file=test-nemlog-in-idp-metadata.xml ``` ### Multiple AuthenticatedFilters and step-up diff --git a/demo/src/main/resources/oiosaml.properties b/demo/src/main/resources/oiosaml.properties index f99f404..8c6c367 100644 --- a/demo/src/main/resources/oiosaml.properties +++ b/demo/src/main/resources/oiosaml.properties @@ -5,9 +5,9 @@ oiosaml.servlet.keystore.alias=java.referenceimplementering # for use with NemLog-in2 oiosaml.servlet.entityid=https://saml.oiosaml3-demo-app oiosaml.servlet.baseurl=https://localhost:8443/oiosaml3-demo.java -# IntTest Idp +# IntTest Idp, metadata downloaded from the NemLog-in test portal and placed on the classpath #oiosaml.servlet.idp.entityid=https://saml.test-nemlog-in.dk/ -#oiosaml.servlet.idp.metadata.url=https://test-nemlog-in.dk/Testportal/Test-nemlog-in-2.xml +#oiosaml.servlet.idp.metadata.file=test-nemlog-in-idp-metadata.xml # DevTest4 Idp oiosaml.servlet.idp.entityid=https://saml.test-devtest4-nemlog-in.dk oiosaml.servlet.idp.metadata.file=test-devtest4-idp-metadata.xml @@ -16,8 +16,8 @@ oiosaml.servlet.idp.metadata.file=test-devtest4-idp-metadata.xml #oiosaml.servlet.entityid=https://saml.oiosaml3-demo-app #oiosaml.servlet.baseurl=https://localhost:8443/oiosaml3-demo.java #oiosaml.servlet.idp.entityid=https://localhost:7080 -#oiosaml.servlet.idp.metadata.url=https://localhost:7080/saml/metadata -#oiosaml.servlet.trust.selfsigned.certs=true +# Fetch https://localhost:7080/saml/metadata once and save it next to this file +#oiosaml.servlet.idp.metadata.file=test-idp-metadata.xml oiosaml.servlet.revocation.crl.check.enabled=false oiosaml.servlet.revocation.ocsp.check.enabled=false 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 2fb8253..6603abd 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 @@ -35,7 +35,6 @@ public class Configuration { // Metadata configuration private String idpEntityID; // This IdP's EntityID - private String idpMetadataUrl; // The URL for the IdP Metadata private String idpMetadataFile; // The file path for a metadata file private int idpMetadataMinRefreshDelay = 1; // The minimum refresh delay in hours private int idpMetadataMaxRefreshDelay = 12; // The maximum refresh delay in hours @@ -57,7 +56,6 @@ public class Configuration { private String logoutPage; private String loginPage; private String nameIDFormat = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"; - private boolean supportSelfSigned = false; // Revocation check settings private boolean crlCheckEnabled = true; @@ -120,14 +118,6 @@ public void setIdpEntityID(String idpEntityID) { this.idpEntityID = idpEntityID; } - public String getIdpMetadataUrl() { - return idpMetadataUrl; - } - - public void setIdpMetadataUrl(String idpMetadataUrl) { - this.idpMetadataUrl = idpMetadataUrl; - } - public String getIdpMetadataFile() { return idpMetadataFile; } @@ -208,14 +198,6 @@ public void setSignatureAlgorithm(String signatureAlgorithm) { this.signatureAlgorithm = signatureAlgorithm; } - public boolean isSupportSelfSigned() { - return supportSelfSigned; - } - - public void setSupportSelfSigned(boolean supportSelfSigned) { - this.supportSelfSigned = supportSelfSigned; - } - public int getClockSkew() { return clockSkew; } @@ -453,7 +435,6 @@ public static class Builder { private String spEntityID; private String baseUrl; private String idpEntityID; - private String idpMetadataUrl; private String idpMetadataFile; private String keystoreLocation; private String keystorePassword; @@ -489,8 +470,8 @@ public Configuration build() throws InternalException { throw new InternalException("Cannot create configuration without IdP's entityID"); } - if (StringUtil.isEmpty(idpMetadataUrl) && StringUtil.isEmpty(idpMetadataFile)) { - throw new InternalException("Cannot create configuration without IdP Metadata URL or File location"); + if (StringUtil.isEmpty(idpMetadataFile)) { + throw new InternalException("Cannot create configuration without the location of the IdP metadata file"); } if (StringUtil.isEmpty(keystoreLocation)) { @@ -510,7 +491,6 @@ public Configuration build() throws InternalException { configuration.spEntityID = this.spEntityID; configuration.baseUrl = this.baseUrl; configuration.idpEntityID = this.idpEntityID; - configuration.idpMetadataUrl = this.idpMetadataUrl; configuration.idpMetadataFile = this.idpMetadataFile; configuration.keystoreLocation = this.keystoreLocation; configuration.keystorePassword = this.keystorePassword; @@ -551,11 +531,6 @@ public Builder setIdpEntityID(String idpEntityID) { return this; } - public Builder setIdpMetadataUrl(String idpMetadataUrl) { - this.idpMetadataUrl = idpMetadataUrl; - return this; - } - public Builder setIdpMetadataFile(String idpMetadataFile) { this.idpMetadataFile = idpMetadataFile; return this; diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java b/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java index 5c0b3e4..1c274b1 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java @@ -1,9 +1,6 @@ package dk.gov.oio.saml.model; import java.io.ByteArrayInputStream; -import java.security.KeyManagementException; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -12,15 +9,8 @@ import java.util.Objects; import java.util.Set; -import javax.net.ssl.SSLContext; import dk.gov.oio.saml.util.ResourceUtil; -import org.apache.http.conn.ssl.NoopHostnameVerifier; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.conn.ssl.TrustSelfSignedStrategy; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.ssl.TrustStrategy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.bouncycastle.util.encoders.Base64; @@ -30,7 +20,6 @@ import org.opensaml.saml.common.xml.SAMLConstants; import org.opensaml.saml.metadata.resolver.impl.AbstractReloadingMetadataResolver; import org.opensaml.saml.metadata.resolver.impl.FilesystemMetadataResolver; -import org.opensaml.saml.metadata.resolver.impl.HTTPMetadataResolver; import org.opensaml.saml.saml2.metadata.EntityDescriptor; import org.opensaml.saml.saml2.metadata.IDPSSODescriptor; import org.opensaml.saml.saml2.metadata.KeyDescriptor; @@ -56,11 +45,9 @@ public class IdPMetadata { private AbstractReloadingMetadataResolver resolver; private DateTime lastCRLCheck; private String entityId; - private String metadataURL; - public IdPMetadata(String entityId, String metadataURL, String metadataFilePath) throws ExternalException, InternalException { + public IdPMetadata(String entityId, String metadataFilePath) throws ExternalException, InternalException { this.entityId = entityId; - this.metadataURL = metadataURL; this.metadataFilePath = metadataFilePath; getEntityDescriptor(); // Fetch metadata first time } @@ -96,27 +83,26 @@ public IDPSSODescriptor getSSODescriptor() throws ExternalException, InternalExc return getEntityDescriptor().getIDPSSODescriptor(SAMLConstants.SAML20P_NS); } - public X509Certificate getValidX509Certificate(UsageType usageType) throws InternalException, ExternalException { + /** + * All certificates the IdP publishes for the given usage that passed revocation checking. + * + *

An IdP publishes both the outgoing and the incoming certificate while it rotates a key, and either + * of them can be the one in use at any moment, so callers have to accept all of them rather than picking + * one.

+ */ + public List getValidX509Certificates(UsageType usageType) throws InternalException, ExternalException { doRevocationCheck(); - X509Certificate result = null; + List result = new ArrayList<>(); if (UsageType.ENCRYPTION.equals(usageType)) { - if (validEncryptionCertificates != null && !validEncryptionCertificates.isEmpty()) { - result = validEncryptionCertificates.get(0); - } + result.addAll(validEncryptionCertificates); } else if (UsageType.SIGNING.equals(usageType)) { - if (validSigningCertificates != null && !validSigningCertificates.isEmpty()) { - result = validSigningCertificates.get(0); - } + result.addAll(validSigningCertificates); } - // If certificate is not found yet, try the unspecified - if (result == null) { - if (validUnspecifiedCertificates != null && !validUnspecifiedCertificates.isEmpty()) { - result = validUnspecifiedCertificates.get(0); - } - } + // Certificates published without a usage serve both purposes + result.addAll(validUnspecifiedCertificates); return result; } @@ -245,28 +231,20 @@ private void initMetadataResolver() throws InternalException, ExternalException try { Configuration config = OIOSAML3Service.getConfig(); - CloseableHttpClient httpClient; - if (config.isSupportSelfSigned()) { - TrustStrategy acceptingTrustStrategy = new TrustSelfSignedStrategy(); - SSLContext sslContext = org.apache.http.ssl.SSLContexts.custom().loadTrustMaterial(null, acceptingTrustStrategy).build(); - SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE); - httpClient = HttpClients.custom().setSSLSocketFactory(csf).build(); - } else { - httpClient = HttpClients.createDefault(); - } - - if (metadataFilePath != null) { - log.debug("MetadataFilePath supplied. Using file based metadata resolver"); - resolver = new FilesystemMetadataResolver(ResourceUtil.getResourceAsFile(metadataFilePath)); - } else { - log.debug("MetadataFilePath not supplied. Using URL based metadata resolver"); - resolver = new HTTPMetadataResolver(httpClient, metadataURL); - } + // Metadata is deployed as a file. The IdP does not sign its metadata, so trust in it comes + // from the deployment of that file, not from the transport it was fetched over + log.debug("Reading IdP metadata from {}", metadataFilePath); + resolver = new FilesystemMetadataResolver(ResourceUtil.getResourceAsFile(metadataFilePath)); resolver.setId(entityId); + + // The file is re-read while the SP runs, so replacing it is enough to publish new keys resolver.setMinRefreshDelay(1000L * 60 * 60 * config.getIdpMetadataMinRefreshDelay()); resolver.setMaxRefreshDelay(1000L * 60 * 60 * config.getIdpMetadataMaxRefreshDelay()); - } catch (ResolverException | KeyManagementException | NoSuchAlgorithmException | KeyStoreException e) { + + // Metadata that has passed its validUntil is not used + resolver.setRequireValidMetadata(true); + } catch (ResolverException e) { throw new InternalException("Could not create MetadataResolver", e); } diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/IdPMetadataService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/IdPMetadataService.java index f071068..63a34c5 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/IdPMetadataService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/IdPMetadataService.java @@ -34,7 +34,7 @@ public IdPMetadata getIdPMetadata() throws ExternalException, InternalException // This method is needed since we only have one IdP functionality for now. Configuration config = OIOSAML3Service.getConfig(); - return getIdPMetadata(config.getIdpEntityID(), config.getIdpMetadataUrl(), config.getIdpMetadataFile()); + return getIdPMetadata(config.getIdpEntityID(), config.getIdpMetadataFile()); } public SingleLogoutService getLogoutEndpoint() throws InternalException, ExternalException { @@ -45,12 +45,12 @@ public String getLogoutResponseEndpoint() throws InternalException, ExternalExce return getIdPMetadata().getLogoutResponseEndpoint(); } - private IdPMetadata getIdPMetadata(String idpEntityID, String idpMetadataURL, String idpMetadataFilePath) throws InternalException, ExternalException { + private IdPMetadata getIdPMetadata(String idpEntityID, String idpMetadataFilePath) throws InternalException, ExternalException { IdPMetadata idPMetadata = identityProviders.get(idpEntityID); // If IdP Metadata has not been fetched before, create object if (idPMetadata == null) { - idPMetadata = new IdPMetadata(idpEntityID, idpMetadataURL, idpMetadataFilePath); + idPMetadata = new IdPMetadata(idpEntityID, idpMetadataFilePath); identityProviders.put(idpEntityID, idPMetadata); } diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java index 75262b6..98fdecc 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java @@ -278,16 +278,25 @@ private void validateInResponseTo(Response response, AuthnRequestWrapper authnRe } private void validateSignature(Assertion assertion) throws ExternalException, InternalException, AssertionValidationException { - // Get Signing credential - X509Certificate x509Certificate = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING); - BasicX509Credential credential = new BasicX509Credential(x509Certificate); + // Get Signing credentials. The IdP publishes every key that may be in use, so the signature is + // accepted when it validates against any of them, not only the first + List x509Certificates = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificates(UsageType.SIGNING); + if (x509Certificates.isEmpty()) { + throw new InternalException("No valid signing certificate found in IdP metadata"); + } // Validate Signature - try { - SignatureValidator.validate(assertion.getSignature(), credential); - } catch (SignatureException e) { - throw new AssertionValidationException("Could not validate assertion signature", e); + SignatureException lastFailure = null; + for (X509Certificate x509Certificate : x509Certificates) { + try { + SignatureValidator.validate(assertion.getSignature(), new BasicX509Credential(x509Certificate)); + return; + } catch (SignatureException e) { + lastFailure = e; + } } + + throw new AssertionValidationException("Could not validate assertion signature", lastFailure); } private void validateAudienceRestriction(Assertion assertion) throws AssertionValidationException { diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java index d416aba..dc8e410 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java @@ -60,11 +60,6 @@ private void handleOptionalValues(Map config, Configuration conf } } - value = config.get(Constants.SUPPORT_SELF_SIGNED); - if (StringUtil.isNotEmpty(value)) { - configuration.setSupportSelfSigned("true".equals(value)); - } - value = config.get(Constants.CRL_CHECK_ENABLED); if (StringUtil.isNotEmpty(value)) { configuration.setCRLCheckEnabled("true".equals(value)); @@ -267,12 +262,39 @@ private Map getInitConfig() { return configMap; } + /** + * Refuse to start on configuration that is no longer supported, rather than starting with the setting + * silently ignored and a different trust model than the deployment expects. + */ + private void rejectRemovedConfiguration(Map config) throws ServletException { + String metadataUrl = config.get(Constants.REMOVED_IDP_METADATA_URL); + if (StringUtil.isNotEmpty(metadataUrl)) { + if (StringUtil.isNotEmpty(config.get(Constants.IDP_METADATA_FILE))) { + log.warn("'{}' is no longer supported and is ignored, IdP metadata is read from '{}'", + Constants.REMOVED_IDP_METADATA_URL, Constants.IDP_METADATA_FILE); + } + else { + throw new ServletException(String.format( + "'%s' is no longer supported. The IdP does not sign its metadata, so trust in it comes from deploying the metadata file. Download the metadata and point '%s' at it", + Constants.REMOVED_IDP_METADATA_URL, Constants.IDP_METADATA_FILE)); + } + } + + if ("true".equals(config.get(Constants.REMOVED_SUPPORT_SELF_SIGNED))) { + throw new ServletException(String.format( + "'%s' is no longer supported. It only relaxed TLS validation for fetching IdP metadata, which is now read from a file", + Constants.REMOVED_SUPPORT_SELF_SIGNED)); + } + } + // Should make sure all handlers are initialized and added to the list private void initServlet() throws ServletException { if (!initialized) { // convert to more useful map Map config = getInitConfig(); + rejectRemovedConfiguration(config); + try { // create configuration with mandatory settings @@ -283,7 +305,6 @@ private void initServlet() throws ServletException { .setKeystorePassword(config.get(Constants.KEYSTORE_PASSWORD)) .setKeyAlias(config.get(Constants.KEY_ALIAS)) .setIdpEntityID(config.get(Constants.IDP_ENTITY_ID)) - .setIdpMetadataUrl(config.get(Constants.IDP_METADATA_URL)) .setIdpMetadataFile(config.get(Constants.IDP_METADATA_FILE)) .setServletRoutingPathPrefix(config.get(Constants.SP_ROUTING_BASE)) .setServletRoutingPathSuffixError(config.get(Constants.SP_ROUTING_ERROR)) diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java b/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java index c080683..1c97751 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java @@ -14,7 +14,9 @@ public class Constants { public static final String KEY_ALIAS = "oiosaml.servlet.keystore.alias"; public static final String IDP_ENTITY_ID = "oiosaml.servlet.idp.entityid"; public static final String IDP_METADATA_FILE = "oiosaml.servlet.idp.metadata.file"; - public static final String IDP_METADATA_URL = "oiosaml.servlet.idp.metadata.url"; + // Removed, IdP metadata is deployed as a file. Kept so a configuration still using it is rejected + // instead of silently ignored, see DispatcherServlet + public static final String REMOVED_IDP_METADATA_URL = "oiosaml.servlet.idp.metadata.url"; // Configuration constants for DispatcherServlet (optional, has default values) public static final String EXTERNAL_CONFIGURATION_FILE = "oiosaml.servlet.configurationfile"; @@ -32,7 +34,7 @@ public class Constants { public static final String ERROR_PAGE = "oiosaml.servlet.secondary.page.error"; public static final String LOGOUT_PAGE = "oiosaml.servlet.secondary.page.logout"; public static final String LOGIN_PAGE = "oiosaml.servlet.secondary.page.login"; - public static final String SUPPORT_SELF_SIGNED = "oiosaml.servlet.trust.selfsigned.certs"; + public static final String REMOVED_SUPPORT_SELF_SIGNED = "oiosaml.servlet.trust.selfsigned.certs"; public static final String SP_ROUTING_BASE = "oiosaml.servlet.routing.path.prefix"; public static final String SP_ROUTING_ERROR = "oiosaml.servlet.routing.path.suffix.error"; public static final String SP_ROUTING_METADATA = "oiosaml.servlet.routing.path.suffix.metadata"; diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/audit/AuditServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/audit/AuditServiceTest.java index 07a42dd..5d11f9d 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/audit/AuditServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/audit/AuditServiceTest.java @@ -49,7 +49,7 @@ void setupConfiguration() throws InternalException { .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setKeystoreLocation(TestConstants.SP_KEYSTORE_LOCATION) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) .setKeyAlias(TestConstants.SP_KEYSTORE_ALIAS) 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 index 283a651..f070b8b 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/config/ConfigurationTest.java @@ -13,7 +13,7 @@ private Configuration minimalConfiguration() throws InternalException { .setSpEntityID("https://sp.example.com") .setBaseUrl("https://sp.example.com") .setIdpEntityID("https://idp.example.com") - .setIdpMetadataUrl("https://idp.example.com/metadata") + .setIdpMetadataFile("test-metadata.xml") .setKeystoreLocation("keystore.p12") .setKeystorePassword("password") .setKeyAlias("alias") diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/filter/AuthenticatedFilterTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/filter/AuthenticatedFilterTest.java index c8af77b..49171ba 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/filter/AuthenticatedFilterTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/filter/AuthenticatedFilterTest.java @@ -57,7 +57,7 @@ public static void beforeAll(MockServerClient idp) throws Exception { .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation(TestConstants.SP_KEYSTORE_LOCATION) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/model/IdPMetadataTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/model/IdPMetadataTest.java index 7f7be5b..389bfd3 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/model/IdPMetadataTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/model/IdPMetadataTest.java @@ -29,7 +29,7 @@ public void testGetLogoutResponseEndpoint_WithEmptyResponseLocation() throws Ext private void testSingleLogoutResponseLocation(String idpMetadataFileLocation, String expectedUri) throws ExternalException, InternalException { ClassLoader classLoader = IdpMetadataServiceTest.class.getClassLoader(); String fileLocation = classLoader.getResource(idpMetadataFileLocation).getFile(); - IdPMetadata idpMetadata = new IdPMetadata("http://mockidp.localhost", null, fileLocation); + IdPMetadata idpMetadata = new IdPMetadata("http://mockidp.localhost", fileLocation); String responseLocation = idpMetadata.getLogoutResponseEndpoint(); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/AuthnRequestServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/AuthnRequestServiceTest.java index 951d43b..d443480 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/AuthnRequestServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/AuthnRequestServiceTest.java @@ -50,7 +50,7 @@ public static void beforeAll(MockServerClient idp) throws Exception { .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation(TestConstants.SP_KEYSTORE_LOCATION) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java index b9d5132..e6b457c 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java @@ -29,7 +29,7 @@ public static void beforeAll(MockServerClient idp) throws Exception { .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation(keystoreLocation) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java index d2d0a3a..1b0f0d2 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java @@ -33,7 +33,7 @@ public void testKeystoreAliasIsCaseInsensitive() throws Exception { .setSpEntityID(TestConstants.SP_ENTITY_ID) .setBaseUrl(TestConstants.SP_BASE_URL) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setKeystoreLocation(keystoreLocation) .setKeystorePassword("Test1234") .setKeyAlias(ALIAS_IN_DIFFERENT_CASE) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/IdpMetadataServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/IdpMetadataServiceTest.java index 5bc87a4..c9d5ca1 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/IdpMetadataServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/IdpMetadataServiceTest.java @@ -52,38 +52,19 @@ public void testGetMetadataFromFile() throws Exception { @DisplayName("Test retrieving metadata") @Test public void testGetMetadata() throws Exception { - // Make sure Mock idp is setup to return correct data - idp - .when( - request() - .withMethod("GET") - .withPath("/saml/metadata"), - Times.exactly(1) - ) - .respond( - response() - .withStatusCode(200) - .withBody(TestConstants.IDP_METADATA)); - + Configuration config = OIOSAML3Service.getConfig(); + config.setIdpMetadataFile(TestConstants.idpMetadataFile()); + EntityDescriptor entityDescriptor = IdPMetadataService.getInstance().getIdPMetadata().getEntityDescriptor(); Assertions.assertNotNull(entityDescriptor); Assertions.assertEquals(TestConstants.IDP_ENTITY_ID, entityDescriptor.getEntityID()); } - + @DisplayName("Test retrieving incorrect metadata") @Test public void testGetIncorrectMetadata() throws Exception { - // Make sure Mock idp is setup to return incorrect data - idp.when( - request() - .withMethod("GET") - .withPath("/saml/metadata"), - Times.exactly(1) - ) - .respond( - response() - .withStatusCode(200) - .withBody(TestConstants.BAD_IDP_METADATA)); + Configuration config = OIOSAML3Service.getConfig(); + config.setIdpMetadataFile(TestConstants.writeIdpMetadataFile(TestConstants.BAD_IDP_METADATA)); // we should get NULL back, if the EntityId does not match EntityDescriptor entityDescriptor = IdPMetadataService.getInstance().getIdPMetadata().getEntityDescriptor(); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/OIOSAML3ServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/OIOSAML3ServiceTest.java index 9ecada2..c6d3550 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/OIOSAML3ServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/OIOSAML3ServiceTest.java @@ -24,7 +24,7 @@ void testInvalidKeystoreConfiguration() throws InternalException, Initialization .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation(TestConstants.SP_KEYSTORE_LOCATION) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) @@ -55,7 +55,7 @@ void testValidConfiguration() throws InternalException, InitializationException .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation(TestConstants.SP_KEYSTORE_LOCATION) .setKeystorePassword(TestConstants.SP_KEYSTORE_PASSWORD) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java index 070fa10..a73aed0 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java @@ -4,6 +4,8 @@ import dk.gov.oio.saml.service.AssertionService; import dk.gov.oio.saml.service.AuthnRequestService; import dk.gov.oio.saml.service.BaseServiceTest; +import dk.gov.oio.saml.service.IdPMetadataService; +import dk.gov.oio.saml.service.OIOSAML3Service; import dk.gov.oio.saml.session.AuthnRequestWrapper; import dk.gov.oio.saml.util.ExternalException; import dk.gov.oio.saml.util.IdpUtil; @@ -58,6 +60,48 @@ public void testValidateCorrectAssertion() throws Exception { validationService.validate(request, messageContext, response, assertion, new AuthnRequestWrapper(authnRequest, NSISLevel.SUBSTANTIAL, "")); } + @DisplayName("Test that validator accepts a signature made with any of the signing certificates in metadata") + @Test + public void testValidateAssertionSignedWithSecondCertificateInMetadata() throws Exception { + AssertionValidationService validationService = new AssertionValidationService(); + + // Metadata where the certificate actually used for signing is preceded by another one, as it is + // while the IdP rotates its signing key + String otherCertificate = IdpUtil.getIdpCertificateBase64(false); + String metadata = TestConstants.IDP_METADATA.replaceFirst("", + "" + + otherCertificate + ""); + + String originalMetadataFile = OIOSAML3Service.getConfig().getIdpMetadataFile(); + OIOSAML3Service.getConfig().setIdpMetadataFile(TestConstants.writeIdpMetadataFile(metadata)); + IdPMetadataService.getInstance().clear(TestConstants.IDP_ENTITY_ID); + + try { + // Mock HttpServletRequest + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL)); + + // Create AuthnRequest + AuthnRequestService authnRequestService = AuthnRequestService.getInstance(); + AuthnRequest authnRequest = getAuthnRequest(authnRequestService); + String inResponseToId = authnRequest.getID(); + + // Create MessageContext, Response and Assertion + String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + MessageContext messageContext = IdpUtil.createMessageWithAssertion(true, true, true, nameID, TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, inResponseToId); + Response response = (Response) messageContext.getMessage(); + + Assertion assertion = new AssertionService().getAssertion(response); + + // Validate + validationService.validate(request, messageContext, response, assertion, new AuthnRequestWrapper(authnRequest, NSISLevel.SUBSTANTIAL, "")); + } + finally { + OIOSAML3Service.getConfig().setIdpMetadataFile(originalMetadataFile); + IdPMetadataService.getInstance().clear(TestConstants.IDP_ENTITY_ID); + } + } + @DisplayName("Test that validator will fail an assertion with the wrong destination") @Test public void testFailAssertionWithWrongDestination() throws Exception { diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/session/SessionDestroyListenerTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/session/SessionDestroyListenerTest.java index b96e313..7072215 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/session/SessionDestroyListenerTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/session/SessionDestroyListenerTest.java @@ -26,7 +26,7 @@ void beforeEach() throws Exception { .setServletRoutingPathSuffixLogoutResponse(TestConstants.SP_ROUTING_LOGOUT_RESPONSE) .setServletRoutingPathSuffixAssertion(TestConstants.SP_ROUTING_ASSERTION) .setIdpEntityID(TestConstants.IDP_ENTITY_ID) - .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setIdpMetadataFile(TestConstants.idpMetadataFile()) .setSessionHandlerFactoryClassName(TestSessionHandlerFactory.class.getName()) .setKeystoreLocation("sp.pfx") .setKeystorePassword("Test1234") diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java index 427dcb7..c58aecc 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java @@ -383,6 +383,14 @@ private static T buildSAMLObject(final Class clazz) { return object; } + /** + * Base64 encoded certificate of the IdP signing key, or of the unrelated key used for invalid + * signatures, for building metadata variants. + */ + public static String getIdpCertificateBase64(boolean validSignature) throws Exception { + return java.util.Base64.getEncoder().encodeToString(getX509Credential(validSignature).getEntityCertificate().getEncoded()); + } + private static BasicX509Credential getX509Credential(boolean validSignature) throws Exception { String resourceName = (validSignature) ? "idp.pfx" : "idp-invalid.pfx"; diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java index 5424a13..7e7b56c 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java @@ -18,6 +18,26 @@ public class TestConstants { public static final String IDP_ENTITY_ID = "http://mockidp.localhost"; public static final String IDP_METADATA_URL = "http://localhost:8081/saml/metadata"; + + /** + * IdP metadata is deployed as a file, so tests hand the library a file rather than a URL. + */ + public static String idpMetadataFile() { + return writeIdpMetadataFile(IDP_METADATA); + } + + public static String writeIdpMetadataFile(String metadata) { + try { + java.io.File file = java.io.File.createTempFile("oiosaml-test-idp-metadata", ".xml"); + file.deleteOnExit(); + java.nio.file.Files.write(file.toPath(), metadata.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + + return file.getAbsolutePath(); + } + catch (java.io.IOException e) { + throw new IllegalStateException("Could not write test metadata file", e); + } + } public static final String IDP_LOGOUT_REQUEST_URL = "http://localhost:8081/saml/logout"; public static final String IDP_LOGOUT_RESPONSE_URL = "http://localhost:8081/saml/logout/response"; From 8a9a1702aa57d59d418379c748eadee65a28bd4c Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 18 Aug 2026 10:10:53 +0200 Subject: [PATCH 2/5] REF-33: Correct the documented lookup rules for the file-reference settings The keystore, IdP metadata and configuration file settings are all resolved from the classpath first and then as a filesystem path, so they were not classpath-only as documented. Also note that metadata packed inside a war or jar is copied to a temporary file at startup, which the periodic refresh then re-reads, and that the configuration file is merged on top of the init-params rather than replacing them. Co-Authored-By: Claude Opus 5 --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 77ab9e4..be25671 100644 --- a/README.md +++ b/README.md @@ -159,12 +159,12 @@ This chapter describes all the configuration parameters and their default values |---------|-----------|---------------|-------------| | `oiosaml.servlet.entityid` | Yes | | The EntityID which identifies the application as a Service Provider, e.g. `http://saml.serviceprovider.com`. | | `oiosaml.servlet.baseurl` | Yes | | The URL on which the application is accessible in a web-browser. The value is used to generate SAML metadata, which must contain login/logout URL endpoints, e.g. `https://serviceprovider.com`. The value above results in generated metadata URLs like `https://serviceprovider.com/saml/assertionConsumer`. | -| `oiosaml.servlet.keystore.location` | Yes | | The name of the PKCS#12 keystore file, located on the classpath of the application. | +| `oiosaml.servlet.keystore.location` | Yes | | A reference to the PKCS#12 keystore file. Resolved from the classpath first, falling back to a filesystem path (absolute, or relative to the working directory), so the keystore can be deployed with the application or held outside it. | | `oiosaml.servlet.keystore.password` | Yes | | The password to the PKCS#12 keystore given above. | | `oiosaml.servlet.keystore.alias` | Yes | | The alias of the key entry in the PKCS#12 keystore given above. | | `oiosaml.servlet.idp.entityid` | Yes | | The EntityID of the SAML Identity Provider that is used for login. | -| `oiosaml.servlet.idp.metadata.file` | Yes | | A FILE reference to the SAML Identity Provider metadata. The file must be located on the classpath of the application. Download the metadata from the Identity Provider and deploy it with the application: NemLog-in does not sign its metadata, so trust in it comes from deploying the file, not from the transport it was fetched over. | -| `oiosaml.servlet.configurationfile` | No | | A FILE reference to a configuration file. If supplied, the `DispatcherServlet` will read its configuration from that file instead of the `ServletConfig` section. See [DispatcherServlet configuration from file](#dispatcherservlet-configuration-from-file). | +| `oiosaml.servlet.idp.metadata.file` | Yes | | A reference to the SAML Identity Provider metadata file. Resolved from the classpath first, falling back to a filesystem path (absolute, or relative to the working directory), so the metadata can be deployed with the application or held outside it. Download the metadata from the Identity Provider and deploy it with the application: NemLog-in does not sign its metadata, so trust in it comes from deploying the file, not from the transport it was fetched over. Note that metadata packed inside a war/jar is read once at startup and copied to a temporary file; for the periodic refresh to pick up changes, the value must resolve to a real filesystem path. | +| `oiosaml.servlet.configurationfile` | No | | A reference to a configuration file. Resolved from the classpath first, falling back to a filesystem path (absolute, or relative to the working directory), so the configuration can be deployed with the application or held outside it. If supplied, the properties in the file are merged on top of the `init-param`s of the `ServletConfig` section, overriding any key given in both places. If the file cannot be read, a warning is logged and the `init-param`s are used unchanged. See [DispatcherServlet configuration from file](#dispatcherservlet-configuration-from-file). | | `oiosaml.servlet.profile.validation.enabled` | No | `true` | By default, the framework performs OIO SAML 3.0 profile validation. If this is not needed, turn off this setting by setting the value to `false`. | | `oiosaml.servlet.profile.validation.assurancelevel.allowed` | No | `false` | The NemLog-in IdP cannot for all authentication provide a NSIS LoA. Therefore the service provider can decide to accept the AssuranceLevel which the NemLog-in IdP provides instead. If this is not acceptable, turn off this setting by omitting it or setting the value to `false`. | | `oiosaml.servlet.profile.validation.assurancelevel.minimum` | No | `3` | If the AssuranceLevel is acceptable, a minimum value can be specified using this setting. Any integer is accepted, however the NemLog-in IdP will never provide an integer larger than 3. | @@ -211,7 +211,7 @@ This chapter describes all the configuration parameters and their default values ### DispatcherServlet configuration from file -If the `oiosaml.servlet.configurationfile` setting is supplied to the `DispatcherServlet`, it will read its configuration from the supplied file instead. This file should be an ordinary property file, supplying properties as key/value pairs like the example below: +If the `oiosaml.servlet.configurationfile` setting is supplied to the `DispatcherServlet`, the properties in that file are merged on top of the `init-param`s, so a key given in both places takes its value from the file. The file is looked up on the classpath first, then as a filesystem path. It should be an ordinary property file, supplying properties as key/value pairs like the example below: ```properties oiosaml.servlet.idp.entityid=https://saml.test-nemlog-in.dk/ From 28803b22e533d975a03b35c0ee18ef98301d0475 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 18 Aug 2026 10:21:00 +0200 Subject: [PATCH 3/5] REF-33: Use the DevTest4 IdP certificate for the revocation tests The certificate the valid-certificate revocation tests used expired on 2026-08-17, so the OCSP test started failing: the PKIX validation rejects it on the validity period, which is not the revoked status the test is about. The DevTest4 IdP signing certificate is issued by the same test CA and runs until 2028. Co-Authored-By: Claude Opus 5 --- .../src/test/java/dk/gov/oio/saml/util/TestConstants.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java index 7e7b56c..9dafad0 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java @@ -168,7 +168,10 @@ public static String writeIdpMetadataFile(String metadata) { ""; public static final String BAD_SP_ASSERTION_CONSUMER_URL = "http://localhost:8080/sso"; - public static final String VALID_CERTIFICATE = "MIIGkzCCBMegAwIBAgIUdxCsIBOOtB5tqNtriG1TMHD9dqYwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yMzA4MTgxMDI2NThaFw0yNjA4MTcxMDI2NTdaMIGpMSUwIwYDVQQDDBxqYXZhLnJlZmVyZW5jZWltcGxlbWVudGVyaW5nMTcwNQYDVQQFEy5VSTpESy1POkc6MmRjZjc5MTktYjI4Mi00NGQxLWI5ODAtM2I3MzcwMGE3ZGQ0MSEwHwYDVQQKDBhEaWdpdGFsaXNlcmluZ3NzdHlyZWxzZW4xFzAVBgNVBGEMDk5UUkRLLTM0MDUxMTc4MQswCQYDVQQGEwJESzCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoCggGBAKNAf9uAhuz3bEjPPFrBa39HCF6S64pSzGRr5yYm3lCBElYJvHzDr9lMKgbv8rKglIVgjWh+PzUjiwIlGjrqAbYa2Hg08Vw2H60GQSFP8rGsshgR+E5Ca2nb9kUcQXAQJl9ScG9squCPRNkdp8vSblRwv/3N0ksjxdZk1wdZ86bOqTsFEjpzhFdBXXSMl4tbhE7WOruKc0QqjUkzXJyp4qwyB2XA75+jsvtRHN/luOzCkUxLhEkFrbg+B6IWqjUuO132xC8d5+T8Y39K6rs4BYOIgQRJOg0OlA5844CC/WBLtAYgMiu1ucZ4mbVWOmm2F86WVRBdmwlN0CFORXihHiYNZfHpA0rPOSncDDMrrGZ7vuvvXxMfIiHlAniSw4eHaEqtaXqwDyNZbfcXgYkszQd7ZV7YMfAjkDo82Qn+Qz+Oc9qq0Syhd9pdUJ/Q26CjFDiaNSg+hDUUJTxowQAktX3AuwBcDeuMoc2yOGmg2xOf/3bIwJSNm8/b+y0wKfY2FwIDAQABo4IBhjCCAYIwDAYDVR0TAQH/BAIwADAfBgNVHSMEGDAWgBR/KJ/ZcZlC4nXn1zV2Lk0IJW12XjB7BggrBgEFBQcBAQRvMG0wQwYIKwYBBQUHMAKGN2h0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY2VzL2lzc3VpbmcvMS9jYWNlcnQvaXNzdWluZy5jZXIwJgYIKwYBBQUHMAGGGmh0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY3NwMCEGA1UdIAQaMBgwCAYGBACPegEBMAwGCiqBUIEpAQEBAwcwOwYIKwYBBQUHAQMELzAtMCsGCCsGAQUFBwsCMB8GBwQAi+xJAQIwFIYSaHR0cHM6Ly91aWQuZ292LmRrMEUGA1UdHwQ+MDwwOqA4oDaGNGh0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY2VzL2lzc3VpbmcvMS9jcmwvaXNzdWluZy5jcmwwHQYDVR0OBBYEFNKFKxc70Coluez0ieqiVuK+a01oMA4GA1UdDwEB/wQEAwIFoDBBBgkqhkiG9w0BAQowNKAPMA0GCWCGSAFlAwQCAQUAoRwwGgYJKoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUAogMCASADggGBAIkUbY8meqO9xQQ2gyMS4rfmqW3bV52YGs07DqG0zuVew7W7RMAJWqDLUj5ltMWK7wULcCBS1tjtxOrvMBCoAE42oQfF/EzLRYKr7VgsMyOgUiTk2t6LvyF5A1OGHOUP3lxQKX3viDURXUeoI4QZ3mxbHUg4sQXdXg2hOEhQOarOhWLdV3MzUkA9ZkwjmycXkbLBVdTbr/fODUU0jeDDlaixKXsGI66qg8Ou86nDkyW7wCxQ9QVwJ5YGogy9ZSc6sLt8XSv3+wFlXD/81EzWfqe5BdWX8cukLtSzdzg3SzJifB4IJ6GIQ58+NVLPEMezwZCLODzVkvdJfyWRxJrDijSVCza515qNW52yfYPYkTb+vdvKcFmwO1gCeK0vT21udVkp1grhNzwb8Cj/tq3OZ+IamZXkjL1go9GzSQQ31IbXHEI/oaPLEeX6j9E8X69wVtSti8SWPw0WgoeOglJM5A6fmlJIGhCBPk2klhH3IIU3+tjuz7iyFHZg7gbPhNHdow=="; + // The DevTest4 NemLog-in IdP signing certificate, taken from + // demo/src/main/resources/test-devtest4-idp-metadata.xml. The revocation tests validate against the + // live test CA, so this must be a certificate the CA still knows and has not expired. + public static final String VALID_CERTIFICATE = "MIIGjDCCBMCgAwIBAgIUaGLv8eOYx3cubJyI3ymai6tAXCowQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yNTA4MjAxMzQxNDBaFw0yODA4MTkxMzQxMzlaMIGhMR0wGwYDVQQDDBROZW1Mb2ctaW4gSWRQIC0gVGVzdDE3MDUGA1UEBRMuVUk6REstTzpHOmEwNDBmZTI2LWNlNzgtNDMyOC1hYzBlLTI5ZWViMTJmNDZkZjEhMB8GA1UECgwYRGlnaXRhbGlzZXJpbmdzc3R5cmVsc2VuMRcwFQYDVQRhDA5OVFJESy0zNDA1MTE3ODELMAkGA1UEBhMCREswggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQCVUzcmIfK0NbJEmAQ7kFOTmXs1mkSHF0LAEFq7Ne8vpNz2H+0F5IyWCwsBi3SByPTAEIClbIT6rLFBoMmIwjfRmAXd68zW7NYwR0N8PcR/hvtoCR1tW2JuKhQlKbWlaYLRvvvS3r5giIjHgEXpmv42caB76eYE3J2qWPaACOIRMXky1vCmbQpEwqJr0S9+QDtMYh5ypVk1zX9rrY2TBbienruISBF1090uuzEQZtfiq9nZaBQAJZFlcYLftepSZXW4QgJjWH6BhMrd4nvcz17WeabG3AhhKHvOUDukJgKMzQd/OcS1tY13P7soMeT8D8h5bCnNDoCfqM/5mgxgxZZXnjJhzOCY+MdrXXBRST2uQLNg0gl/LgqmEKk0izQaMtNauR+5fFHSJMN23Ga6fAmHMlyEj+5JhGeScRvIBp9tPERPhRF0MdEjobbcweMRWqurJYsdo9cIHwrL8WIgQGhQRUrKBu9VPZQTtVs3dn3cxZIJ3axu53hWEKRwqw4UoasCAwEAAaOCAYcwggGDMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUfyif2XGZQuJ159c1di5NCCVtdl4wewYIKwYBBQUHAQEEbzBtMEMGCCsGAQUFBzAChjdodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY2FjZXJ0L2lzc3VpbmcuY2VyMCYGCCsGAQUFBzABhhpodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2NzcDAiBgNVHSAEGzAZMAgGBgQAj3oBATANBgsqgVCBKQEBAQMHATA7BggrBgEFBQcBAwQvMC0wKwYIKwYBBQUHCwIwHwYHBACL7EkBAjAUhhJodHRwczovL3VpZC5nb3YuZGswRQYDVR0fBD4wPDA6oDigNoY0aHR0cDovL2NhMS5jdGktZ292LmRrL29jZXMvaXNzdWluZy8xL2NybC9pc3N1aW5nLmNybDAdBgNVHQ4EFgQU9KSlGZd1HK7yFmo+zohr4oq6LLwwDgYDVR0PAQH/BAQDAgWgMEEGCSqGSIb3DQEBCjA0oA8wDQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiAwIBIAOCAYEAPt4QGIebeY3idMN6xPUQUBwYeiIjhtxqo2O9BHELEQ8Vp9fuBiExT286i1nNbd6TBgFFuUPh2kuwUqAj4YfaG7aSiCToJrTuaxBGlMI9nci2l5VDJ/1/lVVT/OkMLgR9xVXWSi8KFT8U9S+E1THE+G1YSNnt50yRDmqGNtG1XcaXQeu2Q4IcjMc2nf7NcGzGwidsKnBv0nNXwoYnh2ZLWPjQAgQzM3EYdZmEqaCrscYDFRbh4/T3TYjOo64N4rYZWTR+4/poz/29tyfVBWnr3rLuJH2q0kaiFVSXRHXoqOpCDUTVwsULShjmAHcPruQK7mRK1NQcwJtm0HQNWKUxizqc1pA38/9tU2vkQv+OxEEwHnCRK4XkLCJ8ZKCV5+c7AehhTLtDHqsA93+8afgrTLe+kWEO+xRgOJxKV9TnqkvnYiG8BtCOcb4+6yUMGAoOPULVKywIzutRsHYjZptCkuggBqNztSFU/DUd6SpSxf1MGfeh4SC4yPNvKQ+5fo7S"; public static final String REVOKED_CERTIFICATE = "MIIGbjCCBKKgAwIBAgIUSC6sLWUUKtMVPVEu8snwA47QlqgwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yNjA3MDYxMzMyMzZaFw0yOTA3MDUxMzMyMzVaMIGDMRAwDgYDVQQDDAdSZXZva2VkMTcwNQYDVQQFEy5VSTpESy1POkc6Mjc1ODFhMzktYjYxNC00YzYxLWJlY2UtOWIyYjA5Zjg1NzJhMRAwDgYDVQQKDAdpZDIgQS9TMRcwFQYDVQRhDA5OVFJESy00NDE2MDMxNTELMAkGA1UEBhMCREswggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQDWBfNLzMmPCnFq9kRVStjsaGyiN6jO2vNRS/w1VTHW1UWPpdZZ07CsEOKFDgOy6Nd+Ra4wN/cnkE49+Bf/QDJQjZE2gtho+ajELFmrgrtPjscn4Htp+q9rd4X698LmFwc9J00bQMBgZ0Zr2KdWsfp5tYa5JP/Ro3w81FZx/xB7zOm/U1rw7be9gN9TvgWVZHK7z529lAnhVW5KSsJdrckeJvZtxX9EoGeWmeVY3OJM/QjJbXAUkouaizTt54Gfc0mpPXGv+MBK+ufGlQtJJZkdbjifS5HY7EgYD/+WM6SFNzRYeM5TU7+QMBZSlcaSoCCOLKN5/fjlXbkPRsUSx4coj37/LFmAhszkAV8CCotXe/Q6DlNtxWAAEnxmCSrKhMHCdRnEZaSDf4o7zBWl4tKpLbS/fqWC2+gV1euH4pV858uJKsoRmM1eSw5DU0NSnMenreZo0Q8VL9wz1AGnoCycqST5s+deRIJJgtB18NMlGv3LBsIuiEDB35zK8CnyOVUCAwEAAaOCAYcwggGDMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUfyif2XGZQuJ159c1di5NCCVtdl4wewYIKwYBBQUHAQEEbzBtMEMGCCsGAQUFBzAChjdodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY2FjZXJ0L2lzc3VpbmcuY2VyMCYGCCsGAQUFBzABhhpodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2NzcDAiBgNVHSAEGzAZMAgGBgQAj3oBATANBgsqgVCBKQEBAQMHATA7BggrBgEFBQcBAwQvMC0wKwYIKwYBBQUHCwIwHwYHBACL7EkBAjAUhhJodHRwczovL3VpZC5nb3YuZGswRQYDVR0fBD4wPDA6oDigNoY0aHR0cDovL2NhMS5jdGktZ292LmRrL29jZXMvaXNzdWluZy8xL2NybC9pc3N1aW5nLmNybDAdBgNVHQ4EFgQU3TbRHMKCpscjpajDI1TJoOzaPhIwDgYDVR0PAQH/BAQDAgWgMEEGCSqGSIb3DQEBCjA0oA8wDQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiAwIBIAOCAYEAK8HpHPPvXC1IPUVCIfkO0VwnvCBOLrCsr01pziBPg8AV0aVw2gEpyNi0xuIEqS3ipzObHwJu8s99e+rsrFjxCPPAG+Dq8tI1Hl85roZ5sanmLQMZLJMfQzKctImGT/3Q3WSxGGWkLkGObTPZcejLLKNjAfHd5Ufqx7q4wkU7UG/+OpJl8uS3AN2zZMi/39Lgnqraor0AgAmxA5DSv7h06YglgkVrLT4gzd/upTrODaYGia9l8UPGaP+tD0Jc/RRWe59vfGIz36QSu5ceIrgv/t1LQ2H0gl6pzruQ1aZDMYh4pygZbXROGv3UVURNyuftMFWwGE6snJquvlP8mPj9/HW75WD8kl2ART8YT41/BkOFbE6aC+Qgkl46Bx1WpktGElM6NGb4UlBDAwGp2Xin2tVITOuRBPKoAv3I2Fxkh7EBiMmWLZ58l8gCpW2/qOXZAdlnLiDkzc+qvXprYm81T2+Y3eA0LSOpNWOLPe77sjEj0/f92aZSb6+pviML7sDb"; } From 4e6603f8576644af20b3d0f82377590fe3c54a00 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Wed, 26 Aug 2026 21:24:28 +0200 Subject: [PATCH 4/5] Fixed merge conflict, and handle IdP multi signing cert validation correct in LogoutRequestService. --- .../saml/service/LogoutRequestService.java | 19 +- .../AssertionValidationService.java | 26 +-- .../IdPSignatureValidationService.java | 73 ++++++++ .../AssertionValidationServiceTest.java | 4 +- .../servlet/LogoutRequestHandlerTest.java | 175 +++++++++++++++--- 5 files changed, 235 insertions(+), 62 deletions(-) create mode 100644 oiosaml/src/main/java/dk/gov/oio/saml/service/validation/IdPSignatureValidationService.java diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java index 66d4d7f..f2e270b 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java @@ -1,5 +1,7 @@ package dk.gov.oio.saml.service; +import java.util.List; + import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; @@ -19,20 +21,19 @@ import org.opensaml.saml.saml2.metadata.IDPSSODescriptor; import org.opensaml.saml.saml2.metadata.SingleSignOnService; import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator; -import org.opensaml.security.credential.UsageType; +import org.opensaml.security.credential.Credential; import org.opensaml.security.credential.impl.StaticCredentialResolver; -import org.opensaml.security.x509.BasicX509Credential; import org.opensaml.xmlsec.SignatureSigningParameters; import org.opensaml.xmlsec.SignatureValidationParameters; import org.opensaml.xmlsec.context.SecurityParametersContext; import org.opensaml.xmlsec.keyinfo.impl.StaticKeyInfoCredentialResolver; import org.opensaml.xmlsec.signature.Signature; import org.opensaml.xmlsec.signature.support.SignatureException; -import org.opensaml.xmlsec.signature.support.SignatureValidator; import org.opensaml.xmlsec.signature.support.impl.ExplicitKeySignatureTrustEngine; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import dk.gov.oio.saml.service.validation.IdPSignatureValidationService; import dk.gov.oio.saml.util.ExternalException; import dk.gov.oio.saml.util.InternalException; import dk.gov.oio.saml.util.SamlHelper; @@ -78,7 +79,8 @@ private static void validateMessageSignature(LogoutRequest logoutRequest) throws try { // Establishes that the signature is bound to this message, see SAMLSignatureProfileValidator new SAMLSignatureProfileValidator().validate(signature); - SignatureValidator.validate(signature, getIdPSigningCredential()); + + IdPSignatureValidationService.validateSignedByIdP(signature); } catch (SignatureException e) { throw new ExternalException("LogoutRequest signature could not be validated", e); @@ -92,10 +94,11 @@ private static void validateQueryStringSignature(HttpServletRequest httpServletR messageContext.getSubcontext(SAMLProtocolContext.class, true).setProtocol(SAMLConstants.SAML20P_NS); + List idPSigningCredentials = IdPSignatureValidationService.getIdPSigningCredentials(); SignatureValidationParameters validationParameters = new SignatureValidationParameters(); validationParameters.setSignatureTrustEngine(new ExplicitKeySignatureTrustEngine( - new StaticCredentialResolver(getIdPSigningCredential()), - new StaticKeyInfoCredentialResolver(getIdPSigningCredential()))); + new StaticCredentialResolver(idPSigningCredentials), + new StaticKeyInfoCredentialResolver(idPSigningCredentials))); messageContext.getSubcontext(SecurityParametersContext.class, true).setSignatureValidationParameters(validationParameters); SAML2HTTPRedirectDeflateSignatureSecurityHandler signatureHandler = new SAML2HTTPRedirectDeflateSignatureSecurityHandler(); @@ -123,10 +126,6 @@ private static void validateQueryStringSignature(HttpServletRequest httpServletR } } - private static BasicX509Credential getIdPSigningCredential() throws ExternalException, InternalException { - return new BasicX509Credential(IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING)); - } - public static MessageContext createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination, String index) throws InitializationException, InternalException { // Create message context MessageContext messageContext = new MessageContext<>(); diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java index 91bcaee..5939243 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java @@ -1,6 +1,5 @@ package dk.gov.oio.saml.service.validation; -import java.security.cert.X509Certificate; import java.util.List; import java.util.Map; import java.util.Objects; @@ -34,11 +33,8 @@ import org.opensaml.saml.saml2.core.SubjectConfirmation; import org.opensaml.saml.saml2.core.SubjectConfirmationData; import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator; -import org.opensaml.security.credential.UsageType; -import org.opensaml.security.x509.BasicX509Credential; import org.opensaml.xmlsec.signature.Signature; import org.opensaml.xmlsec.signature.support.SignatureException; -import org.opensaml.xmlsec.signature.support.SignatureValidator; import dk.gov.oio.saml.config.Configuration; import dk.gov.oio.saml.model.NSISLevel; @@ -294,25 +290,11 @@ private void validateSignature(Assertion assertion) throws ExternalException, In throw new AssertionValidationException("Assertion signature does not follow the SAML signature profile", e); } - // Get Signing credentials. The IdP publishes every key that may be in use, so the signature is - // accepted when it validates against any of them, not only the first - List x509Certificates = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificates(UsageType.SIGNING); - if (x509Certificates.isEmpty()) { - throw new InternalException("No valid signing certificate found in IdP metadata"); - } - - // Validate Signature - SignatureException lastFailure = null; - for (X509Certificate x509Certificate : x509Certificates) { - try { - SignatureValidator.validate(assertion.getSignature(), new BasicX509Credential(x509Certificate)); - return; - } catch (SignatureException e) { - lastFailure = e; - } + try { + IdPSignatureValidationService.validateSignedByIdP(signature); + } catch (SignatureException e) { + throw new AssertionValidationException("Could not validate assertion signature", e); } - - throw new AssertionValidationException("Could not validate assertion signature", lastFailure); } private void validateAudienceRestriction(Assertion assertion) throws AssertionValidationException { diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/IdPSignatureValidationService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/IdPSignatureValidationService.java new file mode 100644 index 0000000..d46bbea --- /dev/null +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/validation/IdPSignatureValidationService.java @@ -0,0 +1,73 @@ +package dk.gov.oio.saml.service.validation; + +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.List; + +import org.opensaml.security.credential.Credential; +import org.opensaml.security.credential.UsageType; +import org.opensaml.security.x509.BasicX509Credential; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.SignatureException; +import org.opensaml.xmlsec.signature.support.SignatureValidator; + +import dk.gov.oio.saml.service.IdPMetadataService; +import dk.gov.oio.saml.util.ExternalException; +import dk.gov.oio.saml.util.InternalException; + +/** + * Signatures made by the IdP, validated against the signing keys it publishes in its metadata. + */ +public class IdPSignatureValidationService { + + private IdPSignatureValidationService() { + } + + /** + * Every signing key the IdP publishes, as credentials. + * + *

The IdP publishes both the outgoing and the incoming key while it rotates, and either of them can be + * the one in use at any moment, so callers have to accept a signature made with any of them rather than + * with the first alone.

+ * + * @throws InternalException if the metadata holds no usable signing certificate, since no signature from + * the IdP could then be validated at all + */ + public static List getIdPSigningCredentials() throws ExternalException, InternalException { + List certificates = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificates(UsageType.SIGNING); + if (certificates.isEmpty()) { + throw new InternalException("No valid signing certificate found in IdP metadata"); + } + + List credentials = new ArrayList<>(certificates.size()); + for (X509Certificate certificate : certificates) { + credentials.add(new BasicX509Credential(certificate)); + } + + return credentials; + } + + /** + * Validate that a signature was made with one of the signing keys the IdP publishes. + * + *

This establishes the key alone. That the signature also covers the element about to be consumed is a + * separate property, established by SAMLSignatureProfileValidator, and callers need both.

+ * + * @throws SignatureException if the signature was made with none of the published keys + */ + public static void validateSignedByIdP(Signature signature) throws SignatureException, ExternalException, InternalException { + SignatureException lastFailure = null; + + for (Credential credential : getIdPSigningCredentials()) { + try { + SignatureValidator.validate(signature, credential); + return; + } + catch (SignatureException e) { + lastFailure = e; + } + } + + throw new SignatureException("Signature was not made with any of the signing keys published by the IdP", lastFailure); + } +} diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java index b5dda34..6bb7759 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java @@ -461,8 +461,8 @@ public void testFailSignatureNotBoundToAssertion() throws Exception { // Only interesting while the signature itself still verifies, otherwise the test would pass for the // wrong reason - X509Certificate idpCertificate = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING); - SignatureValidator.validate(unboundAssertion.getSignature(), new BasicX509Credential(idpCertificate)); + List idpCertificates = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificates(UsageType.SIGNING); + SignatureValidator.validate(unboundAssertion.getSignature(), new BasicX509Credential(idpCertificates.get(0))); // Validate, should fail because the signature is not bound to the assertion being consumed AssertionValidationException exception = Assertions.assertThrows(AssertionValidationException.class, () -> { diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java index ccac42f..e178fc6 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java @@ -32,12 +32,15 @@ import org.w3c.dom.Element; import dk.gov.oio.saml.model.NSISLevel; +import dk.gov.oio.saml.service.IdPMetadataService; import dk.gov.oio.saml.service.OIOSAML3Service; import net.shibboleth.utilities.java.support.codec.Base64Support; import net.shibboleth.utilities.java.support.xml.SerializeSupport; public class LogoutRequestHandlerTest { + private static final String NAME_ID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + @DisplayName("Test that a logged-in user can perform a logout") @Test public void testLogoutRequestWhenLoggedIn() throws InternalException, IOException, ExternalException, URISyntaxException { @@ -50,7 +53,7 @@ public void testLogoutRequestWhenLoggedIn() throws InternalException, IOExceptio // Mock session with state: not logged in at any NSIS level Mockito.when(sessionHandler.isAuthenticated(session)).thenReturn(true); Mockito.when(assertionWrapper.getNsisLevel()).thenReturn(NSISLevel.SUBSTANTIAL); - Mockito.when(assertionWrapper.getSubjectNameId()).thenReturn("https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"); + Mockito.when(assertionWrapper.getSubjectNameId()).thenReturn(NAME_ID); Mockito.when(assertionWrapper.getSubjectNameIdFormat()).thenReturn(NameID.PERSISTENT); // Mock HttpServletRequest @@ -66,7 +69,7 @@ public void testLogoutRequestWhenLoggedIn() throws InternalException, IOExceptio LogoutRequestHandler logoutRequestHandler = new LogoutRequestHandler(); logoutRequestHandler.handleGet(request, response); - Mockito.verify(sessionHandler).logout(session,assertionWrapper); + Mockito.verify(sessionHandler).logout(session, assertionWrapper); Mockito.verify(session).invalidate(); Mockito.verify(response).sendRedirect(Mockito.anyString()); @@ -93,9 +96,9 @@ public void testIdPLogoutRequestWhenLoggedIn() throws Exception { SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler(); // Create LogoutRequest - String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + String nameID = NAME_ID; MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); - String sessionIndex = ((LogoutRequest)messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); + String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); // Marshall and serialize Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage()); @@ -143,7 +146,7 @@ public void testIdPLogoutRequestWhenLoggedIn() throws Exception { logoutRequestHandler.handleGet(request, response); // Verification - Mockito.verify(sessionHandler).logout(session,assertionWrapper); + Mockito.verify(sessionHandler).logout(session, assertionWrapper); Mockito.verify(session).invalidate(); Mockito.verify(outputStreamMock).flush(); //Verify that something is sent to the IdP Mockito.verify(logoutRequestHandler).sendPost(Mockito.eq(response), contextArgumentCaptor.capture()); @@ -166,9 +169,9 @@ public void testIdPSOAPLogoutRequestWhenLoggedIn() throws Exception { SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler(); // Create LogoutRequest - String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + String nameID = NAME_ID; MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); - String sessionIndex = ((LogoutRequest)messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); + String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); // Marshall and serialize Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage()); @@ -193,7 +196,7 @@ public void testIdPSOAPLogoutRequestWhenLoggedIn() throws Exception { Mockito.when(request.getMethod()).thenReturn("POST"); // Method: GET Mockito.when(request.getContentType()).thenReturn("text/xml"); Mockito.when(request.getHeader("SOAPAction")).thenReturn("SOAPAction"); - Mockito.when(request.getInputStream()).thenReturn(new ServletInputStream(){ + Mockito.when(request.getInputStream()).thenReturn(new ServletInputStream() { public int read() throws IOException { return inputStream.read(); } @@ -231,7 +234,7 @@ public void setReadListener(ReadListener readListener) { logoutRequestHandler.handleSOAP(request, response); // Verification - Mockito.verify(sessionHandler).logout(session,assertionWrapper); + Mockito.verify(sessionHandler).logout(session, assertionWrapper); Mockito.verify(session).invalidate(); Mockito.verify(outputStreamMock).flush(); //Verify that something is sent to the IdP Mockito.verify(logoutRequestHandler).sendSOAP(Mockito.eq(response), contextArgumentCaptor.capture()); @@ -245,7 +248,7 @@ public void setReadListener(ReadListener readListener) { Assertions.assertEquals(TestConstants.SP_ENTITY_ID, logoutResponse.getIssuer().getValue()); Assertions.assertEquals(TestConstants.IDP_LOGOUT_RESPONSE_URL, logoutResponse.getDestination()); } - + @DisplayName("Test that an IdP can request a logout with a signature on the query string") @Test public void testIdPLogoutRequestSignedOnQueryString() throws Exception { @@ -255,7 +258,7 @@ public void testIdPLogoutRequestSignedOnQueryString() throws Exception { // Create LogoutRequest without a signature on the message itself, the HTTP-Redirect binding signs // the query string instead - String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + String nameID = NAME_ID; MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true); String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); String redirectUrl = IdpUtil.encodeAsRedirectUrl(messageContext); @@ -286,30 +289,57 @@ public void testIdPLogoutRequestSignedOnQueryString() throws Exception { @Test public void testRejectUnsignedLogoutRequest() throws Exception { assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest( - "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7", - NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true)); + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true)); } @DisplayName("Test that a LogoutRequest signed with an unknown key is rejected") @Test public void testRejectLogoutRequestSignedWithUnknownKey() throws Exception { assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest( - "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7", - NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, false)); + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, false)); } @DisplayName("Test that a LogoutRequest from another issuer is rejected") @Test public void testRejectLogoutRequestFromUnknownIssuer() throws Exception { assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest( - "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7", - NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, true, "https://not-the-configured-idp")); + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, true, "https://not-the-configured-idp")); + } + + @DisplayName("Test that a LogoutRequest signed with any of the signing certificates in metadata is accepted") + @Test + public void testLogoutRequestSignedWithSecondCertificateInMetadata() throws Exception { + MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest( + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); + + withIdPMetadata(metadataWithTwoSigningCertificates(), () -> assertLogoutRequestAccepted(messageContext, false)); + } + + @DisplayName("Test that a query string signed with any of the signing certificates in metadata is accepted") + @Test + public void testLogoutRequestSignedOnQueryStringWithSecondCertificateInMetadata() throws Exception { + MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest( + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true); + + withIdPMetadata(metadataWithTwoSigningCertificates(), () -> assertLogoutRequestAccepted(messageContext, true)); + } + + @DisplayName("Test that a LogoutRequest is rejected when metadata holds no signing certificate") + @Test + public void testRejectLogoutRequestWhenMetadataHasNoSigningCertificate() throws Exception { + MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest( + NAME_ID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); + + // Without a certificate there is nothing to validate the signature against, so the request cannot be + // accepted on the grounds that no validation failed + withIdPMetadata(metadataWithoutSigningCertificate(), + () -> assertLogoutRequestRejected(messageContext, InternalException.class)); } /** - * Send the LogoutRequest to the handler and require that it is refused without any session being touched. + * Send the LogoutRequest to the handler and require that the session it names is logged out. */ - private void assertLogoutRequestRejected(MessageContext messageContext) throws Exception { + private void assertLogoutRequestAccepted(MessageContext messageContext, boolean signedOnQueryString) throws Exception { HttpSession session = Mockito.mock(HttpSession.class); AssertionWrapper assertionWrapper = Mockito.mock(AssertionWrapper.class); SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler(); @@ -319,24 +349,113 @@ private void assertLogoutRequestRejected(MessageContext messageConte Mockito.when(sessionHandler.isAuthenticated(session)).thenReturn(true); Mockito.when(sessionHandler.getAuthnRequest(session)).thenReturn(null); - // Marshall, deflate and base64 encode as the HTTP-Redirect binding does + // Mock HttpServletRequest + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL)); + Mockito.when(request.getSession()).thenReturn(session); + Mockito.when(request.getMethod()).thenReturn("GET"); + + if (signedOnQueryString) { + IdpUtil.stubRedirectRequest(request, IdpUtil.encodeAsRedirectUrl(messageContext)); + } else { + Mockito.when(request.getParameter("RelayState")).thenReturn(null); + Mockito.when(request.getParameter("SAMLRequest")).thenReturn(deflateAndEncode(messageContext)); + } + + // Mock HttpServletResponse + ServletOutputStream outputStreamMock = Mockito.mock(ServletOutputStream.class); + HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + Mockito.when(response.getOutputStream()).thenReturn(outputStreamMock); + + new LogoutRequestHandler().handleGet(request, response); + + Mockito.verify(sessionHandler).logout(session, assertionWrapper); + Mockito.verify(session).invalidate(); + } + + /** + * Run the action with the library pointed at the given IdP metadata, as a deployment is while the IdP + * rotates its signing key. + */ + private void withIdPMetadata(String metadata, TestAction action) throws Exception { + String originalMetadataFile = OIOSAML3Service.getConfig().getIdpMetadataFile(); + OIOSAML3Service.getConfig().setIdpMetadataFile(TestConstants.writeIdpMetadataFile(metadata)); + IdPMetadataService.getInstance().clear(TestConstants.IDP_ENTITY_ID); + + try { + action.run(); + } finally { + OIOSAML3Service.getConfig().setIdpMetadataFile(originalMetadataFile); + IdPMetadataService.getInstance().clear(TestConstants.IDP_ENTITY_ID); + } + } + + @FunctionalInterface + private interface TestAction { + void run() throws Exception; + } + + /** + * Marshall, deflate and base64 encode the message as the HTTP-Redirect binding does. + */ + private static String deflateAndEncode(MessageContext messageContext) throws Exception { Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage()); + ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); DeflaterOutputStream deflaterStream = new DeflaterOutputStream(bytesOut, new Deflater(8, true)); deflaterStream.write(SerializeSupport.nodeToString(marshalledMessage).getBytes("UTF-8")); deflaterStream.finish(); + return Base64Support.encode(bytesOut.toByteArray(), Base64Support.UNCHUNKED); + } + + /** + * Metadata where the certificate actually used for signing is preceded by another one, as it is while the + * IdP rotates its signing key. + */ + private static String metadataWithTwoSigningCertificates() throws Exception { + String otherCertificate = IdpUtil.getIdpCertificateBase64(false); + + return TestConstants.IDP_METADATA.replaceFirst("", + "" + + otherCertificate + ""); + } + + /** + * Metadata publishing no key for signing, as it is when every published certificate has been revoked. + */ + private static String metadataWithoutSigningCertificate() { + return TestConstants.IDP_METADATA.replace("", ""); + } + + /** + * Send the LogoutRequest to the handler and require that it is refused without any session being touched. + */ + private void assertLogoutRequestRejected(MessageContext messageContext) throws Exception { + assertLogoutRequestRejected(messageContext, ExternalException.class); + } + + private void assertLogoutRequestRejected(MessageContext messageContext, Class expected) throws Exception { + HttpSession session = Mockito.mock(HttpSession.class); + AssertionWrapper assertionWrapper = Mockito.mock(AssertionWrapper.class); + SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler(); + + String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); + Mockito.when(sessionHandler.getAssertion(sessionIndex)).thenReturn(assertionWrapper); + Mockito.when(sessionHandler.isAuthenticated(session)).thenReturn(true); + Mockito.when(sessionHandler.getAuthnRequest(session)).thenReturn(null); + // Mock HttpServletRequest HttpServletRequest request = Mockito.mock(HttpServletRequest.class); Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL)); Mockito.when(request.getSession()).thenReturn(session); Mockito.when(request.getMethod()).thenReturn("GET"); Mockito.when(request.getParameter("RelayState")).thenReturn(null); - Mockito.when(request.getParameter("SAMLRequest")).thenReturn(Base64Support.encode(bytesOut.toByteArray(), Base64Support.UNCHUNKED)); + Mockito.when(request.getParameter("SAMLRequest")).thenReturn(deflateAndEncode(messageContext)); HttpServletResponse response = Mockito.mock(HttpServletResponse.class); - Assertions.assertThrows(ExternalException.class, () -> new LogoutRequestHandler().handleGet(request, response)); + Assertions.assertThrows(expected, () -> new LogoutRequestHandler().handleGet(request, response)); // The session handler mock is shared between tests, so verify against this tests own session Mockito.verify(sessionHandler, Mockito.never()).logout(Mockito.eq(session), Mockito.any(AssertionWrapper.class)); @@ -376,9 +495,9 @@ public void testLogoutRequestWhenNotLoggedIn() throws InternalException, IOExcep @Test public void testSOAPLogoutRequestWhenNotLoggedIn() throws Exception { // Create LogoutRequest - String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + String nameID = NAME_ID; MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); - String sessionIndex = ((LogoutRequest)messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); + String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); // Marshall and serialize Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage()); @@ -402,7 +521,7 @@ public void testSOAPLogoutRequestWhenNotLoggedIn() throws Exception { Mockito.when(request.getMethod()).thenReturn("POST"); // Method: GET Mockito.when(request.getContentType()).thenReturn("text/xml"); Mockito.when(request.getHeader("SOAPAction")).thenReturn("SOAPAction"); - Mockito.when(request.getInputStream()).thenReturn(new ServletInputStream(){ + Mockito.when(request.getInputStream()).thenReturn(new ServletInputStream() { public int read() throws IOException { return inputStream.read(); } @@ -461,9 +580,9 @@ public void setReadListener(ReadListener readListener) { @Test public void testIdPLogoutRequestWhenNotLoggedIn() throws Exception { // Create LogoutRequest - String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + String nameID = NAME_ID; MessageContext messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL); - String sessionIndex = ((LogoutRequest)messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); + String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex(); // Marshall and serialize Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage()); @@ -506,7 +625,7 @@ public void testIdPLogoutRequestWhenNotLoggedIn() throws Exception { Mockito.verify(sessionHandler, Mockito.never()).getAssertion(session); Mockito.verify(sessionHandler, Mockito.times(1)).getAssertion(sessionIndex); - Mockito.verify(sessionHandler, Mockito.never()).logout(Mockito.eq(session),Mockito.any(AssertionWrapper.class)); + Mockito.verify(sessionHandler, Mockito.never()).logout(Mockito.eq(session), Mockito.any(AssertionWrapper.class)); Mockito.verify(session).invalidate(); Mockito.verify(outputStreamMock).flush(); //Verify that something is sent to the IdP } From 04233a301827c2ef82f8fecf512e7822d60f673f Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Wed, 26 Aug 2026 21:33:30 +0200 Subject: [PATCH 5/5] Improved comment to explain what deprecated configuration now prevents DispatcherServlet to initialize. --- .../java/dk/gov/oio/saml/servlet/DispatcherServlet.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java index dc8e410..5edda2f 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java @@ -263,8 +263,13 @@ private Map getInitConfig() { } /** - * Refuse to start on configuration that is no longer supported, rather than starting with the setting - * silently ignored and a different trust model than the deployment expects. + * Do not start on if not supported configuration is provided: + *

+ * We no longer allow 'oiosaml.servlet.idp.metadata.url' to be specified - IdP metadata must be downloaded and deployed + * as it provides the only trust anchor. + *

+ * Configuration 'oiosaml.servlet.trust.selfsigned.certs' is also removed which was used for relaxing TLS trust + * when retrieving IdP metadata over TLS. */ private void rejectRemovedConfiguration(Map config) throws ServletException { String metadataUrl = config.get(Constants.REMOVED_IDP_METADATA_URL);