From 7dde7cffda9b03859c954eb06cfb09baa3bbb54f Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 5 Jan 2021 17:32:47 -0700 Subject: [PATCH] Add Status Check Closes gh-8955 --- .../security/saml2/core/Saml2ErrorCodes.java | 9 ++++++- .../OpenSamlAuthenticationProvider.java | 19 +++++++++++++- .../OpenSamlAuthenticationProviderTests.java | 22 +++++++++++++++- .../authentication/TestOpenSamlObjects.java | 25 ++++++++++++++++++- 4 files changed, 71 insertions(+), 4 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/Saml2ErrorCodes.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/Saml2ErrorCodes.java index 63753f1121..c5cfda1a47 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/Saml2ErrorCodes.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/Saml2ErrorCodes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,13 @@ public interface Saml2ErrorCodes { */ String MALFORMED_RESPONSE_DATA = "malformed_response_data"; + /** + * Response is invalid in a general way. + * + * @since 5.5 + */ + String INVALID_RESPONSE = "invalid_response"; + /** * Response destination does not match the request URL. A SAML 2 response object was * received at a URL that did not match the URL stored in the {code Destination} diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java index 7dc99d5000..46bc579bbf 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -73,6 +73,7 @@ import org.opensaml.saml.saml2.core.EncryptedAttribute; import org.opensaml.saml.saml2.core.NameID; import org.opensaml.saml.saml2.core.OneTimeUse; import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.StatusCode; import org.opensaml.saml.saml2.core.SubjectConfirmation; import org.opensaml.saml.saml2.core.impl.ResponseUnmarshaller; import org.opensaml.saml.saml2.encryption.Decrypter; @@ -615,6 +616,12 @@ public final class OpenSamlAuthenticationProvider implements AuthenticationProvi Response response = responseToken.getResponse(); Saml2AuthenticationToken token = responseToken.getToken(); Saml2ResponseValidatorResult result = Saml2ResponseValidatorResult.success(); + String statusCode = getStatusCode(response); + if (!StatusCode.SUCCESS.equals(statusCode)) { + String message = String.format("Invalid status [%s] for SAML response [%s]", statusCode, + response.getID()); + result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, message)); + } String issuer = response.getIssuer().getValue(); String destination = response.getDestination(); String location = token.getRelyingPartyRegistration().getAssertionConsumerServiceLocation(); @@ -637,6 +644,16 @@ public final class OpenSamlAuthenticationProvider implements AuthenticationProvi }; } + private String getStatusCode(Response response) { + if (response.getStatus() == null) { + return StatusCode.SUCCESS; + } + if (response.getStatus().getStatusCode() == null) { + return StatusCode.SUCCESS; + } + return response.getStatus().getStatusCode().getValue(); + } + private Converter createDefaultAssertionSignatureValidator() { return createAssertionValidator(Saml2ErrorCodes.INVALID_SIGNATURE, (assertionToken) -> { SignatureTrustEngine engine = this.signatureTrustEngineConverter.convert(assertionToken.token); diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProviderTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProviderTests.java index fd24ebd511..3e9f1995ba 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProviderTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -48,6 +48,7 @@ import org.opensaml.saml.saml2.core.EncryptedID; import org.opensaml.saml.saml2.core.NameID; import org.opensaml.saml.saml2.core.OneTimeUse; import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.StatusCode; import org.opensaml.saml.saml2.core.impl.EncryptedAssertionBuilder; import org.opensaml.saml.saml2.core.impl.EncryptedIDBuilder; import org.opensaml.saml.saml2.core.impl.NameIDBuilder; @@ -557,6 +558,25 @@ public class OpenSamlAuthenticationProviderTests { assertThat(authentication.getName()).isEqualTo("decrypted name"); } + @Test + public void authenticateWhenResponseStatusIsNotSuccessThenFails() { + Response response = TestOpenSamlObjects.signedResponseWithOneAssertion( + (r) -> r.setStatus(TestOpenSamlObjects.status(StatusCode.AUTHN_FAILED))); + Saml2AuthenticationToken token = token(response, verifying(registration())); + assertThatExceptionOfType(Saml2AuthenticationException.class) + .isThrownBy(() -> this.provider.authenticate(token)) + .satisfies(errorOf(Saml2ErrorCodes.INVALID_RESPONSE)); + } + + @Test + public void authenticateWhenResponseStatusIsSuccessThenSucceeds() { + Response response = TestOpenSamlObjects + .signedResponseWithOneAssertion((r) -> r.setStatus(TestOpenSamlObjects.successStatus())); + Saml2AuthenticationToken token = token(response, verifying(registration())); + Authentication authentication = this.provider.authenticate(token); + assertThat(authentication.getName()).isEqualTo("test@saml.user"); + } + private T build(QName qName) { return (T) XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilder(qName).buildObject(qName); } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java index 120b031906..40e1d8bfe1 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Base64; import java.util.List; import java.util.UUID; +import java.util.function.Consumer; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; @@ -59,11 +60,15 @@ import org.opensaml.saml.saml2.core.EncryptedID; import org.opensaml.saml.saml2.core.Issuer; import org.opensaml.saml.saml2.core.NameID; import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.Status; +import org.opensaml.saml.saml2.core.StatusCode; import org.opensaml.saml.saml2.core.Subject; import org.opensaml.saml.saml2.core.SubjectConfirmation; import org.opensaml.saml.saml2.core.SubjectConfirmationData; import org.opensaml.saml.saml2.core.impl.AttributeBuilder; import org.opensaml.saml.saml2.core.impl.AttributeStatementBuilder; +import org.opensaml.saml.saml2.core.impl.StatusBuilder; +import org.opensaml.saml.saml2.core.impl.StatusCodeBuilder; import org.opensaml.saml.saml2.encryption.Encrypter; import org.opensaml.security.SecurityException; import org.opensaml.security.credential.BasicCredential; @@ -118,8 +123,14 @@ public final class TestOpenSamlObjects { } static Response signedResponseWithOneAssertion() { + return signedResponseWithOneAssertion((response) -> { + }); + } + + static Response signedResponseWithOneAssertion(Consumer responseConsumer) { Response response = response(); response.getAssertions().add(assertion()); + responseConsumer.accept(response); return signed(response, TestSaml2X509Credentials.assertingPartySigningCredential(), RELYING_PARTY_ENTITY_ID); } @@ -338,6 +349,18 @@ public final class TestOpenSamlObjects { return attributeStatements; } + static Status successStatus() { + return status(StatusCode.SUCCESS); + } + + static Status status(String code) { + Status status = new StatusBuilder().buildObject(); + StatusCode statusCode = new StatusCodeBuilder().buildObject(); + statusCode.setValue(code); + status.setStatusCode(statusCode); + return status; + } + static T build(QName qName) { return (T) XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilder(qName).buildObject(qName); }