From e50c2a6a74d5e25d90d114a665ccd1c83cb6a6f4 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 30 Apr 2026 21:18:42 -0600 Subject: [PATCH 1/2] Rework Login Validation Logic This commit simplifies the validation code, favoring the construction of a list of error codes over using an accumulating Saml2ResponseValidationResult. This will simplify future analysis by making the code more readable. Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com> --- .../BaseOpenSamlAuthenticationProvider.java | 61 +++++++++++-------- .../OpenSaml4AuthenticationProviderTests.java | 51 +++++++++++++++- .../OpenSaml5AuthenticationProviderTests.java | 49 ++++++++++++++- 3 files changed, 135 insertions(+), 26 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java index 5c24acf2c7..8532c62fe4 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java @@ -326,62 +326,75 @@ class BaseOpenSamlAuthenticationProvider implements AuthenticationProvider { boolean responseSigned = response.isSigned(); ResponseToken responseToken = new ResponseToken(response, token); - Saml2ResponseValidatorResult result = this.responseSignatureValidator.convert(responseToken); + Collection responseSignatureErrors = this.responseSignatureValidator.convert(responseToken) + .getErrors(); + if (!responseSignatureErrors.isEmpty()) { + reportErrors(response, responseSignatureErrors); + return; + } + + Collection errors = new ArrayList<>(); if (responseSigned) { this.responseElementsDecrypter.accept(responseToken); } else if (!response.getEncryptedAssertions().isEmpty()) { - result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, + errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, "Did not decrypt response [" + response.getID() + "] since it is not signed")); } if (!this.validateResponseAfterAssertions) { - result = result.concat(this.responseValidator.convert(responseToken)); + errors.addAll(this.responseValidator.convert(responseToken).getErrors()); } boolean allAssertionsSigned = true; for (Assertion assertion : response.getAssertions()) { AssertionToken assertionToken = new AssertionToken(assertion, token); - result = result.concat(this.assertionSignatureValidator.convert(assertionToken)); + Collection assertionSignatureErrors = this.assertionSignatureValidator.convert(assertionToken) + .getErrors(); + errors.addAll(assertionSignatureErrors); allAssertionsSigned = allAssertionsSigned && assertion.isSigned(); + if (!assertionSignatureErrors.isEmpty()) { + continue; + } if (responseSigned || assertion.isSigned()) { this.assertionElementsDecrypter.accept(new AssertionToken(assertion, token)); } - result = result.concat(this.assertionValidator.convert(assertionToken)); + errors.addAll(this.assertionValidator.convert(assertionToken).getErrors()); } if (!responseSigned && !allAssertionsSigned) { String description = "Either the response or one of the assertions is unsigned. " + "Please either sign the response or all of the assertions."; - result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, description)); + errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, description)); } if (this.validateResponseAfterAssertions) { - result = result.concat(this.responseValidator.convert(responseToken)); + errors.addAll(this.responseValidator.convert(responseToken).getErrors()); } else { Assertion firstAssertion = CollectionUtils.firstElement(response.getAssertions()); if (firstAssertion != null && !hasName(firstAssertion)) { - Saml2Error error = new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, - "Assertion [" + firstAssertion.getID() + "] is missing a subject"); - result = result.concat(error); + errors.add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, + "Assertion [" + firstAssertion.getID() + "] is missing a subject")); } } - if (result.hasErrors()) { - Collection errors = result.getErrors(); - if (this.logger.isTraceEnabled()) { - this.logger.trace("Found " + errors.size() + " validation errors in SAML response [" + response.getID() - + "]: " + errors); - } - else if (this.logger.isDebugEnabled()) { - this.logger - .debug("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + "]"); - } - Saml2Error first = errors.iterator().next(); - throw createAuthenticationException(first.getErrorCode(), first.getDescription(), null); - } - else { + reportErrors(response, errors); + } + + private void reportErrors(Response response, Collection errors) { + if (errors.isEmpty()) { if (this.logger.isDebugEnabled()) { this.logger.debug("Successfully processed SAML Response [" + response.getID() + "]"); } + return; } + if (this.logger.isTraceEnabled()) { + this.logger.debug("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + + "]: " + errors); + } + else if (this.logger.isDebugEnabled()) { + this.logger + .debug("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + "]"); + } + Saml2Error first = errors.iterator().next(); + throw createAuthenticationException(first.getErrorCode(), first.getDescription(), null); } private Converter createDefaultResponseSignatureValidator() { diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java index d9a4fd5e25..56af63f186 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java @@ -33,6 +33,7 @@ import javax.xml.namespace.QName; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.core.xml.schema.XSDateTime; @@ -87,6 +88,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; /** * Tests for {@link OpenSaml4AuthenticationProvider} @@ -173,6 +175,53 @@ public class OpenSaml4AuthenticationProviderTests { .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); } + @Test + public void authenticateWhenResponseSignatureInvalidThenSkipsResponseAndAssertionValidation() { + Response response = response(); + response.setDestination(DESTINATION + "invalid"); + Assertion assertion = assertion(); + response.getAssertions().add(signed(assertion)); + // Sign the response with a credential the verifier does not trust + TestOpenSamlObjects.signed(response, TestSaml2X509Credentials.relyingPartyDecryptingCredential(), + RELYING_PARTY_ENTITY_ID); + OpenSaml4AuthenticationProvider provider = new OpenSaml4AuthenticationProvider(); + Converter responseValidator = mock(Converter.class); + Converter assertionValidator = mock( + Converter.class); + provider.setResponseValidator(responseValidator); + provider.setAssertionValidator(assertionValidator); + Saml2AuthenticationToken token = token(response, verifying(registration())); + assertThatExceptionOfType(Saml2AuthenticationException.class).isThrownBy(() -> provider.authenticate(token)) + .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); + verifyNoInteractions(responseValidator, assertionValidator); + } + + @Test + public void authenticateWhenAssertionSignatureInvalidThenSkipsThatAssertionValidationOnly() { + Response response = response(); + Assertion bad = assertion(); + bad.setID("bad-assertion"); + TestOpenSamlObjects.signed(bad, TestSaml2X509Credentials.relyingPartyDecryptingCredential(), + RELYING_PARTY_ENTITY_ID); + response.getAssertions().add(bad); + Assertion good = assertion(); + good.setID("good-assertion"); + response.getAssertions().add(signed(good)); + OpenSaml4AuthenticationProvider provider = new OpenSaml4AuthenticationProvider(); + Converter assertionValidator = mock( + Converter.class); + given(assertionValidator.convert(any(OpenSaml4AuthenticationProvider.AssertionToken.class))) + .willReturn(Saml2ResponseValidatorResult.success()); + provider.setAssertionValidator(assertionValidator); + Saml2AuthenticationToken token = token(signed(response), verifying(registration())); + assertThatExceptionOfType(Saml2AuthenticationException.class).isThrownBy(() -> provider.authenticate(token)) + .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); + ArgumentCaptor captor = ArgumentCaptor + .forClass(OpenSaml4AuthenticationProvider.AssertionToken.class); + verify(assertionValidator).convert(captor.capture()); + assertThat(captor.getValue().getAssertion().getID()).isEqualTo("good-assertion"); + } + @Test public void authenticateWhenOpenSAMLValidationErrorThenThrowAuthenticationException() { Response response = response(); @@ -502,7 +551,7 @@ public class OpenSaml4AuthenticationProviderTests { EncryptedAssertion encryptedAssertion = TestOpenSamlObjects.encrypted(assertion(), TestSaml2X509Credentials.assertingPartyEncryptingCredential()); response.getEncryptedAssertions().add(encryptedAssertion); - Saml2AuthenticationToken token = token(signed(response), registration() + Saml2AuthenticationToken token = token(signed(response), verifying(registration()) .decryptionX509Credentials((c) -> c.add(TestSaml2X509Credentials.assertingPartyPrivateCredential()))); assertThatExceptionOfType(Saml2AuthenticationException.class) .isThrownBy(() -> this.provider.authenticate(token)) diff --git a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java index 309f04bb16..df42a6f381 100644 --- a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java +++ b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java @@ -34,6 +34,7 @@ import javax.xml.namespace.QName; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.core.xml.schema.XSDateTime; @@ -183,6 +184,52 @@ public class OpenSaml5AuthenticationProviderTests { .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); } + @Test + public void authenticateWhenResponseSignatureInvalidThenSkipsResponseAndAssertionValidation() { + Response response = response(); + response.setDestination(DESTINATION + "invalid"); + Assertion assertion = assertion(); + response.getAssertions().add(signed(assertion)); + TestOpenSamlObjects.signed(response, TestSaml2X509Credentials.relyingPartyDecryptingCredential(), + RELYING_PARTY_ENTITY_ID); + OpenSaml5AuthenticationProvider provider = new OpenSaml5AuthenticationProvider(); + Converter responseValidator = mock(Converter.class); + Converter assertionValidator = mock( + Converter.class); + provider.setResponseValidator(responseValidator); + provider.setAssertionValidator(assertionValidator); + Saml2AuthenticationToken token = token(response, verifying(registration())); + assertThatExceptionOfType(Saml2AuthenticationException.class).isThrownBy(() -> provider.authenticate(token)) + .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); + verifyNoInteractions(responseValidator, assertionValidator); + } + + @Test + public void authenticateWhenAssertionSignatureInvalidThenSkipsThatAssertionValidationOnly() { + Response response = response(); + Assertion bad = assertion(); + bad.setID("bad-assertion"); + TestOpenSamlObjects.signed(bad, TestSaml2X509Credentials.relyingPartyDecryptingCredential(), + RELYING_PARTY_ENTITY_ID); + response.getAssertions().add(bad); + Assertion good = assertion(); + good.setID("good-assertion"); + response.getAssertions().add(signed(good)); + OpenSaml5AuthenticationProvider provider = new OpenSaml5AuthenticationProvider(); + Converter assertionValidator = mock( + Converter.class); + given(assertionValidator.convert(any(OpenSaml5AuthenticationProvider.AssertionToken.class))) + .willReturn(Saml2ResponseValidatorResult.success()); + provider.setAssertionValidator(assertionValidator); + Saml2AuthenticationToken token = token(signed(response), verifying(registration())); + assertThatExceptionOfType(Saml2AuthenticationException.class).isThrownBy(() -> provider.authenticate(token)) + .satisfies(errorOf(Saml2ErrorCodes.INVALID_SIGNATURE)); + ArgumentCaptor captor = ArgumentCaptor + .forClass(OpenSaml5AuthenticationProvider.AssertionToken.class); + verify(assertionValidator).convert(captor.capture()); + assertThat(captor.getValue().getAssertion().getID()).isEqualTo("good-assertion"); + } + @Test public void authenticateWhenOpenSAMLValidationErrorThenThrowAuthenticationException() { Response response = response(); @@ -512,7 +559,7 @@ public class OpenSaml5AuthenticationProviderTests { EncryptedAssertion encryptedAssertion = TestOpenSamlObjects.encrypted(assertion(), TestSaml2X509Credentials.assertingPartyEncryptingCredential()); response.getEncryptedAssertions().add(encryptedAssertion); - Saml2AuthenticationToken token = token(signed(response), registration() + Saml2AuthenticationToken token = token(signed(response), verifying(registration()) .decryptionX509Credentials((c) -> c.add(TestSaml2X509Credentials.assertingPartyPrivateCredential()))); assertThatExceptionOfType(Saml2AuthenticationException.class) .isThrownBy(() -> this.provider.authenticate(token)) From cf0687120024c3dad09261dd9df4971b83717a53 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 30 Apr 2026 21:18:01 -0600 Subject: [PATCH 2/2] Rework Logout Validation Logic This commit simplifies the code, favoring the construction of a list of error codes over composing a set of consumers. This will simplify future analysis by making the code more readable. Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com> --- .../BaseOpenSamlLogoutRequestValidator.java | 126 +++++++------- .../BaseOpenSamlLogoutResponseValidator.java | 160 ++++++++---------- .../OpenSamlLogoutRequestValidator.java | 126 +++++++------- .../OpenSamlLogoutResponseValidator.java | 160 +++++++++--------- .../OpenSaml4LogoutRequestValidatorTests.java | 33 ++++ ...OpenSaml4LogoutResponseValidatorTests.java | 50 +++++- .../OpenSamlLogoutRequestValidatorTests.java | 33 ++++ .../OpenSamlLogoutResponseValidatorTests.java | 50 +++++- .../OpenSaml5LogoutRequestValidatorTests.java | 33 ++++ ...OpenSaml5LogoutResponseValidatorTests.java | 50 +++++- 10 files changed, 509 insertions(+), 312 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java index 8fb3c4c62b..b8b021ad97 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.saml.saml2.core.LogoutRequest; import org.opensaml.saml.saml2.core.NameID; @@ -56,84 +57,74 @@ class BaseOpenSamlLogoutRequestValidator implements Saml2LogoutRequestValidator LogoutRequest logoutRequest = this.saml.deserialize(Saml2Utils.withEncoded(request.getSamlRequest()) .inflate(request.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(request, logoutRequest, registration)) - .errors(validateRequest(logoutRequest, registration, authentication)) - .build(); + Collection errors = verifySignature(request, logoutRequest, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutRequest, registration, authentication); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, + private Collection verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, RelyingPartyRegistration registration) { AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); Collection credentials = details.getVerificationX509Credentials(); VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); - return (errors) -> { - if (logoutRequest.isSigned()) { - errors.addAll(verify.verify(logoutRequest)); - } - else { - RedirectParameters params = new RedirectParameters(request.getParameters(), - request.getParametersQuery(), logoutRequest); - errors.addAll(verify.verify(params)); - } - }; + if (logoutRequest.isSigned()) { + return verify.verify(logoutRequest); + } + RedirectParameters params = new RedirectParameters(request.getParameters(), request.getParametersQuery(), + logoutRequest); + return verify.verify(params); } - private Consumer> validateRequest(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - validateIssuer(request, registration).accept(errors); - validateDestination(request, registration).accept(errors); - validateSubject(request, registration, authentication).accept(errors); - }; + private Collection validateRequest(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(request, registration)); + errors.addAll(validateDestination(request, registration)); + errors.addAll(validateSubject(request, registration, authentication)); + return errors; } - private Consumer> validateIssuer(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); - return; - } - String issuer = request.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateIssuer(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); + } + String issuer = request.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateDestination(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutRequest")); - return; - } - String destination = request.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateDestination(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getDestination() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, "Failed to find destination in LogoutRequest")); + } + String destination = request.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateSubject(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - if (authentication == null) { - return; - } - NameID nameId = getNameId(request, registration); - if (nameId == null) { - errors - .add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); - return; - } - - validateNameId(nameId, authentication, errors); - }; + private Collection validateSubject(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + if (authentication == null) { + return Collections.emptyList(); + } + NameID nameId = getNameId(request, registration); + if (nameId == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); + } + return validateNameId(nameId, authentication); } private NameID getNameId(LogoutRequest request, RelyingPartyRegistration registration) { @@ -141,12 +132,13 @@ class BaseOpenSamlLogoutRequestValidator implements Saml2LogoutRequestValidator return request.getNameID(); } - private void validateNameId(NameID nameId, Authentication authentication, Collection errors) { + private Collection validateNameId(NameID nameId, Authentication authentication) { String name = nameId.getValue(); if (!name.equals(authentication.getName())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, "Failed to match subject in LogoutRequest with currently logged in user")); } + return Collections.emptyList(); } } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java index 92305d8610..73480fbdf9 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.saml.saml2.core.LogoutResponse; import org.opensaml.saml.saml2.core.StatusCode; @@ -52,101 +53,90 @@ class BaseOpenSamlLogoutResponseValidator implements Saml2LogoutResponseValidato LogoutResponse logoutResponse = this.saml.deserialize(Saml2Utils.withEncoded(response.getSamlResponse()) .inflate(response.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(response, logoutResponse, registration)) - .errors(validateRequest(logoutResponse, registration)) - .errors(validateLogoutRequest(logoutResponse, request.getId())) - .build(); + Collection errors = verifySignature(response, logoutResponse, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutResponse, registration, request.getId()); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutResponse response, - LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); - Collection credentials = details.getVerificationX509Credentials(); - VerificationConfigurer verify = this.saml.withVerificationKeys(credentials) - .entityId(details.getEntityId()) - .entityId(details.getEntityId()); - if (logoutResponse.isSigned()) { - errors.addAll(verify.verify(logoutResponse)); - } - else { - RedirectParameters params = new RedirectParameters(response.getParameters(), - response.getParametersQuery(), logoutResponse); - errors.addAll(verify.verify(params)); - } - }; - } - - private Consumer> validateRequest(LogoutResponse response, + private Collection verifySignature(Saml2LogoutResponse response, LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - validateIssuer(response, registration).accept(errors); - validateDestination(response, registration).accept(errors); - validateStatus(response).accept(errors); - }; + AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); + Collection credentials = details.getVerificationX509Credentials(); + VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); + if (logoutResponse.isSigned()) { + return verify.verify(logoutResponse); + } + RedirectParameters params = new RedirectParameters(response.getParameters(), response.getParametersQuery(), + logoutResponse); + return verify.verify(params); } - private Consumer> validateIssuer(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); - return; - } - String issuer = response.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateRequest(LogoutResponse response, RelyingPartyRegistration registration, + String logoutRequestId) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(response, registration)); + errors.addAll(validateDestination(response, registration)); + errors.addAll(validateStatus(response)); + errors.addAll(validateLogoutRequest(response, logoutRequestId)); + return errors; } - private Consumer> validateDestination(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutResponse")); - return; - } - String destination = response.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateIssuer(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); + } + String issuer = response.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateStatus(LogoutResponse response) { - return (errors) -> { - if (response.getStatus() == null) { - return; - } - if (response.getStatus().getStatusCode() == null) { - return; - } - if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); - }; + private Collection validateDestination(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getDestination() == null) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to find destination in LogoutResponse")); + } + String destination = response.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateLogoutRequest(LogoutResponse response, String id) { - return (errors) -> { - if (response.getInResponseTo() == null) { - return; - } - if (response.getInResponseTo().equals(id)) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, - "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); - }; + private Collection validateStatus(LogoutResponse response) { + if (response.getStatus() == null) { + return Collections.emptyList(); + } + if (response.getStatus().getStatusCode() == null) { + return Collections.emptyList(); + } + if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + return Collections + .singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); + } + + private Collection validateLogoutRequest(LogoutResponse response, String id) { + if (response.getInResponseTo() == null) { + return Collections.emptyList(); + } + if (response.getInResponseTo().equals(id)) { + return Collections.emptyList(); + } + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, + "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); } } diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidator.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidator.java index 59c35c00f3..870d5bdf4d 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidator.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.saml.saml2.core.LogoutRequest; import org.opensaml.saml.saml2.core.NameID; @@ -68,84 +69,74 @@ public final class OpenSamlLogoutRequestValidator implements Saml2LogoutRequestV LogoutRequest logoutRequest = this.saml.deserialize(Saml2Utils.withEncoded(request.getSamlRequest()) .inflate(request.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(request, logoutRequest, registration)) - .errors(validateRequest(logoutRequest, registration, authentication)) - .build(); + Collection errors = verifySignature(request, logoutRequest, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutRequest, registration, authentication); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, + private Collection verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, RelyingPartyRegistration registration) { AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); Collection credentials = details.getVerificationX509Credentials(); VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); - return (errors) -> { - if (logoutRequest.isSigned()) { - errors.addAll(verify.verify(logoutRequest)); - } - else { - RedirectParameters params = new RedirectParameters(request.getParameters(), - request.getParametersQuery(), logoutRequest); - errors.addAll(verify.verify(params)); - } - }; + if (logoutRequest.isSigned()) { + return verify.verify(logoutRequest); + } + RedirectParameters params = new RedirectParameters(request.getParameters(), request.getParametersQuery(), + logoutRequest); + return verify.verify(params); } - private Consumer> validateRequest(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - validateIssuer(request, registration).accept(errors); - validateDestination(request, registration).accept(errors); - validateSubject(request, registration, authentication).accept(errors); - }; + private Collection validateRequest(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(request, registration)); + errors.addAll(validateDestination(request, registration)); + errors.addAll(validateSubject(request, registration, authentication)); + return errors; } - private Consumer> validateIssuer(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); - return; - } - String issuer = request.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateIssuer(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); + } + String issuer = request.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateDestination(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutRequest")); - return; - } - String destination = request.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateDestination(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getDestination() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, "Failed to find destination in LogoutRequest")); + } + String destination = request.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateSubject(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - if (authentication == null) { - return; - } - NameID nameId = getNameId(request, registration); - if (nameId == null) { - errors - .add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); - return; - } - - validateNameId(nameId, authentication, errors); - }; + private Collection validateSubject(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + if (authentication == null) { + return Collections.emptyList(); + } + NameID nameId = getNameId(request, registration); + if (nameId == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); + } + return validateNameId(nameId, authentication); } private NameID getNameId(LogoutRequest request, RelyingPartyRegistration registration) { @@ -153,12 +144,13 @@ public final class OpenSamlLogoutRequestValidator implements Saml2LogoutRequestV return request.getNameID(); } - private void validateNameId(NameID nameId, Authentication authentication, Collection errors) { + private Collection validateNameId(NameID nameId, Authentication authentication) { String name = nameId.getValue(); if (!name.equals(authentication.getName())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, "Failed to match subject in LogoutRequest with currently logged in user")); } + return Collections.emptyList(); } } diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidator.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidator.java index 4d21f3acea..f42612eb73 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidator.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.core.config.ConfigurationService; import org.opensaml.core.xml.config.XMLObjectProviderRegistry; @@ -59,7 +60,7 @@ public class OpenSamlLogoutResponseValidator implements Saml2LogoutResponseValid private final OpenSamlOperations saml = new OpenSaml4Template(); /** - * Constructs a {@link OpenSamlLogoutRequestValidator} + * Constructs an {@link OpenSamlLogoutResponseValidator} */ public OpenSamlLogoutResponseValidator() { this.registry = ConfigurationService.get(XMLObjectProviderRegistry.class); @@ -78,99 +79,90 @@ public class OpenSamlLogoutResponseValidator implements Saml2LogoutResponseValid LogoutResponse logoutResponse = this.saml.deserialize(Saml2Utils.withEncoded(response.getSamlResponse()) .inflate(response.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(response, logoutResponse, registration)) - .errors(validateRequest(logoutResponse, registration)) - .errors(validateLogoutRequest(logoutResponse, request.getId())) - .build(); + Collection errors = verifySignature(response, logoutResponse, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutResponse, registration, request.getId()); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutResponse response, - LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); - Collection credentials = details.getVerificationX509Credentials(); - VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); - if (logoutResponse.isSigned()) { - errors.addAll(verify.verify(logoutResponse)); - } - else { - RedirectParameters params = new RedirectParameters(response.getParameters(), - response.getParametersQuery(), logoutResponse); - errors.addAll(verify.verify(params)); - } - }; - } - - private Consumer> validateRequest(LogoutResponse response, + private Collection verifySignature(Saml2LogoutResponse response, LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - validateIssuer(response, registration).accept(errors); - validateDestination(response, registration).accept(errors); - validateStatus(response).accept(errors); - }; + AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); + Collection credentials = details.getVerificationX509Credentials(); + VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); + if (logoutResponse.isSigned()) { + return verify.verify(logoutResponse); + } + RedirectParameters params = new RedirectParameters(response.getParameters(), response.getParametersQuery(), + logoutResponse); + return verify.verify(params); } - private Consumer> validateIssuer(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); - return; - } - String issuer = response.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateRequest(LogoutResponse response, RelyingPartyRegistration registration, + String logoutRequestId) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(response, registration)); + errors.addAll(validateDestination(response, registration)); + errors.addAll(validateStatus(response)); + errors.addAll(validateLogoutRequest(response, logoutRequestId)); + return errors; } - private Consumer> validateDestination(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutResponse")); - return; - } - String destination = response.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateIssuer(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); + } + String issuer = response.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateStatus(LogoutResponse response) { - return (errors) -> { - if (response.getStatus() == null) { - return; - } - if (response.getStatus().getStatusCode() == null) { - return; - } - if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); - }; + private Collection validateDestination(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getDestination() == null) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to find destination in LogoutResponse")); + } + String destination = response.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateLogoutRequest(LogoutResponse response, String id) { - return (errors) -> { - if (response.getInResponseTo() == null) { - return; - } - if (response.getInResponseTo().equals(id)) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, - "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); - }; + private Collection validateStatus(LogoutResponse response) { + if (response.getStatus() == null) { + return Collections.emptyList(); + } + if (response.getStatus().getStatusCode() == null) { + return Collections.emptyList(); + } + if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + return Collections + .singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); + } + + private Collection validateLogoutRequest(LogoutResponse response, String id) { + if (response.getInResponseTo() == null) { + return Collections.emptyList(); + } + if (response.getInResponseTo().equals(id)) { + return Collections.emptyList(); + } + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, + "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); } } diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutRequestValidatorTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutRequestValidatorTests.java index 3e77d0f70f..3c4c1ced60 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutRequestValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutRequestValidatorTests.java @@ -26,6 +26,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutRequest; import org.springframework.security.core.Authentication; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -90,6 +91,38 @@ public class OpenSaml4LogoutRequestValidatorTests { assertThat(result.hasErrors()).isFalse(); } + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateRequestFurther() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + sign(logoutRequest, registration); + logoutRequest.getIssuer().setValue("https://other-asserting-party.example"); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("someone-else"); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.validator.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationAndNameIdInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("wrong-user"); + sign(logoutRequest, registration); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.validator.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_REQUEST); + } + @Test public void handleWhenInvalidIssuerThenInvalidSignatureError() { RelyingPartyRegistration registration = registration().build(); diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutResponseValidatorTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutResponseValidatorTests.java index 0a84829704..9c21b1bbab 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutResponseValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4LogoutResponseValidatorTests.java @@ -24,6 +24,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutResponse; import org.opensaml.saml.saml2.core.StatusCode; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -57,7 +58,8 @@ public class OpenSaml4LogoutResponseValidatorTests { Saml2LogoutResponse response = post(logoutResponse, registration); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } @Test @@ -73,7 +75,48 @@ public class OpenSaml4LogoutResponseValidatorTests { this.saml.withSigningKeys(registration.getSigningX509Credentials())); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); + } + + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateResponseFurther() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + sign(logoutResponse, registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationStatusAndInResponseToInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + sign(logoutResponse, registration); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_RESPONSE, + Saml2ErrorCodes.INVALID_RESPONSE); } @Test @@ -145,7 +188,8 @@ public class OpenSaml4LogoutResponseValidatorTests { .build(); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } private RelyingPartyRegistration.Builder registration() { diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidatorTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidatorTests.java index ebb8dab07c..ffc3ee3951 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutRequestValidatorTests.java @@ -26,6 +26,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutRequest; import org.springframework.security.core.Authentication; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -90,6 +91,38 @@ public class OpenSamlLogoutRequestValidatorTests { assertThat(result.hasErrors()).isFalse(); } + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateRequestFurther() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + sign(logoutRequest, registration); + logoutRequest.getIssuer().setValue("https://other-asserting-party.example"); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("someone-else"); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationAndNameIdInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("wrong-user"); + sign(logoutRequest, registration); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_REQUEST); + } + @Test public void handleWhenInvalidIssuerThenInvalidSignatureError() { RelyingPartyRegistration registration = registration().build(); diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidatorTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidatorTests.java index 5b9eeed6dd..72e0a63e04 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlLogoutResponseValidatorTests.java @@ -24,6 +24,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutResponse; import org.opensaml.saml.saml2.core.StatusCode; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -57,7 +58,8 @@ public class OpenSamlLogoutResponseValidatorTests { Saml2LogoutResponse response = post(logoutResponse, registration); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } @Test @@ -73,7 +75,48 @@ public class OpenSamlLogoutResponseValidatorTests { this.saml.withSigningKeys(registration.getSigningX509Credentials())); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); + } + + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateResponseFurther() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + sign(logoutResponse, registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationStatusAndInResponseToInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + sign(logoutResponse, registration); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_RESPONSE, + Saml2ErrorCodes.INVALID_RESPONSE); } @Test @@ -145,7 +188,8 @@ public class OpenSamlLogoutResponseValidatorTests { .build(); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } private RelyingPartyRegistration.Builder registration() { diff --git a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutRequestValidatorTests.java b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutRequestValidatorTests.java index 0bb4ff9f36..7421b9dc6b 100644 --- a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutRequestValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutRequestValidatorTests.java @@ -26,6 +26,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutRequest; import org.springframework.security.core.Authentication; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -90,6 +91,38 @@ public class OpenSaml5LogoutRequestValidatorTests { assertThat(result.hasErrors()).isFalse(); } + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateRequestFurther() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + sign(logoutRequest, registration); + logoutRequest.getIssuer().setValue("https://other-asserting-party.example"); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("someone-else"); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.validator.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationAndNameIdInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); + logoutRequest.setDestination("https://wrong-destination.example"); + logoutRequest.getNameID().setValue("wrong-user"); + sign(logoutRequest, registration); + Saml2LogoutRequest request = post(logoutRequest, registration); + Saml2LogoutRequestValidatorParameters parameters = new Saml2LogoutRequestValidatorParameters(request, + registration, authentication(registration)); + Saml2LogoutValidatorResult result = this.validator.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_REQUEST); + } + @Test public void handleWhenInvalidIssuerThenInvalidSignatureError() { RelyingPartyRegistration registration = registration().build(); diff --git a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutResponseValidatorTests.java b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutResponseValidatorTests.java index e081bafc9d..6bae7e565c 100644 --- a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutResponseValidatorTests.java +++ b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5LogoutResponseValidatorTests.java @@ -24,6 +24,7 @@ import org.opensaml.core.xml.XMLObject; import org.opensaml.saml.saml2.core.LogoutResponse; import org.opensaml.saml.saml2.core.StatusCode; +import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.core.TestSaml2X509Credentials; @@ -57,7 +58,8 @@ public class OpenSaml5LogoutResponseValidatorTests { Saml2LogoutResponse response = post(logoutResponse, registration); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } @Test @@ -73,7 +75,48 @@ public class OpenSaml5LogoutResponseValidatorTests { this.saml.withSigningKeys(registration.getSigningX509Credentials())); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); + } + + @Test + public void handleWhenSignatureVerificationFailsThenDoesNotValidateResponseFurther() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + sign(logoutResponse, registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isTrue(); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsOnly(Saml2ErrorCodes.INVALID_SIGNATURE); + } + + @Test + public void handleWhenDestinationStatusAndInResponseToInvalidThenCollectsAllApplicableRequestErrors() { + RelyingPartyRegistration registration = registration().build(); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .id("id") + .build(); + LogoutResponse logoutResponse = TestOpenSamlObjects.assertingPartyLogoutResponse(registration); + logoutResponse.setDestination("https://wrong-destination.example"); + logoutResponse.getStatus().getStatusCode().setValue(StatusCode.UNKNOWN_PRINCIPAL); + logoutResponse.setInResponseTo("wrong-id"); + sign(logoutResponse, registration); + Saml2LogoutResponse response = post(logoutResponse, registration); + Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, + logoutRequest, registration); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.getErrors()).extracting(Saml2Error::getErrorCode) + .containsExactly(Saml2ErrorCodes.INVALID_DESTINATION, Saml2ErrorCodes.INVALID_RESPONSE, + Saml2ErrorCodes.INVALID_RESPONSE); } @Test @@ -145,7 +188,8 @@ public class OpenSaml5LogoutResponseValidatorTests { .build(); Saml2LogoutResponseValidatorParameters parameters = new Saml2LogoutResponseValidatorParameters(response, logoutRequest, registration); - this.manager.validate(parameters); + Saml2LogoutValidatorResult result = this.manager.validate(parameters); + assertThat(result.hasErrors()).isFalse(); } private RelyingPartyRegistration.Builder registration() {