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 3ac6f0845b..c278b594c3 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 @@ -319,62 +319,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 new Saml2AuthenticationException(first); - } - 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.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 new Saml2AuthenticationException(first); } private Converter createDefaultResponseSignatureValidator() { 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 ca902cb14b..6a5dadf0c6 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; @@ -57,84 +58,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) { @@ -142,13 +133,14 @@ 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 = (authentication.getCredentials() instanceof Saml2ResponseAssertionAccessor assertion) ? assertion.getNameId() : authentication.getName(); if (!nameId.getValue().equals(name)) { - 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/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 01eeafbe16..8744047d95 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 @@ -33,6 +33,7 @@ import java.util.function.Consumer; import javax.xml.namespace.QName; 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; @@ -185,6 +186,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(); @@ -513,7 +560,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)) 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 8c36a79c99..078563e9d9 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 b5b04995e1..42a1c67834 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() {