Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Settings for Destination URL Validation #371

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ onelogin.saml2.security.allow_duplicated_attribute_name = false
# (In order to validate the xml, 'strict' and 'wantXMLValidation' must be true).
onelogin.saml2.security.want_xml_validation = true

# Indicates if the SP will validate 'Destination' attribute in SAML response
# Both 'strict' and 'wantDestinationUrlValidation' must be true
onelogin.saml2.security.want_destination_url_validation = true

# Algorithm that the toolkit will use on signing process. Options:
# 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
# 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ protected void validateAudiences() throws XPathExpressionException, ValidationEr
* @throws ValidationError
*/
protected void validateDestination(final Element element) throws ValidationError {
if (element.hasAttribute("Destination")) {
if (settings.getWantDestinationUrlValidation() && element.hasAttribute("Destination")) {
final String destinationUrl = element.getAttribute("Destination");
if (destinationUrl != null) {
if (destinationUrl.isEmpty()) {
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class Saml2Settings {
private List<String> requestedAuthnContext = new ArrayList<>();
private String requestedAuthnContextComparison = "exact";
private boolean wantXMLValidation = true;
private boolean wantDestinationUrlValidation = true;
private String signatureAlgorithm = Constants.RSA_SHA1;
private String digestAlgorithm = Constants.SHA1;
private boolean rejectUnsolicitedResponsesWithInResponseTo = false;
Expand Down Expand Up @@ -346,6 +347,13 @@ public boolean getWantXMLValidation() {
return wantXMLValidation;
}

/**
* @return the wantDestinationUrlValidation setting value
*/
public boolean getWantDestinationUrlValidation() {
return wantDestinationUrlValidation;
}

/**
* @return the signatureAlgorithm setting value
*/
Expand Down Expand Up @@ -776,6 +784,17 @@ public void setWantXMLValidation(boolean wantXMLValidation) {
this.wantXMLValidation = wantXMLValidation;
}

/**
* Set the wantDestinationUrlValidation setting value
*
* @param wantDestinationUrlValidation
* the wantDestinationUrlValidation value to be set.
* Based on it the SP will validate Destination attribute in SAML response XML.
*/
public void setWantDestinationUrlValidation(boolean wantDestinationUrlValidation) {
this.wantDestinationUrlValidation = wantDestinationUrlValidation;
}

/**
* Set the signatureAlgorithm setting value
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ public class SettingsBuilder {
public final static String SECURITY_REQUESTED_AUTHNCONTEXT = "onelogin.saml2.security.requested_authncontext";
public final static String SECURITY_REQUESTED_AUTHNCONTEXTCOMPARISON = "onelogin.saml2.security.requested_authncontextcomparison";
public final static String SECURITY_WANT_XML_VALIDATION = "onelogin.saml2.security.want_xml_validation";
public final static String SECURITY_WANT_DESTINATION_URL_VALIDATION = "onelogin.saml2.security.want_destination_url_validation";
public final static String SECURITY_SIGNATURE_ALGORITHM = "onelogin.saml2.security.signature_algorithm";
public final static String SECURITY_DIGEST_ALGORITHM = "onelogin.saml2.security.digest_algorithm";
public final static String SECURITY_REJECT_UNSOLICITED_RESPONSES_WITH_INRESPONSETO = "onelogin.saml2.security.reject_unsolicited_responses_with_inresponseto";
Expand Down Expand Up @@ -399,6 +400,10 @@ private void loadSecuritySetting() {
if (wantXMLValidation != null)
saml2Setting.setWantXMLValidation(wantXMLValidation);

Boolean wantDestinationUrlvalidation = loadBooleanProperty(SECURITY_WANT_DESTINATION_URL_VALIDATION);
if (wantDestinationUrlvalidation != null)
saml2Setting.setWantDestinationUrlValidation(wantDestinationUrlvalidation);

Boolean signMetadata = loadBooleanProperty(SECURITY_SIGN_METADATA);
if (signMetadata != null)
saml2Setting.setSignMetadata(signMetadata);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import com.onelogin.saml2.exception.SettingsException;
import com.onelogin.saml2.exception.ValidationError;
import com.onelogin.saml2.http.HttpRequest;
import com.onelogin.saml2.logout.LogoutRequest;
import com.onelogin.saml2.model.SamlResponseStatus;
import com.onelogin.saml2.settings.Saml2Settings;
import com.onelogin.saml2.settings.SettingsBuilder;
Expand Down Expand Up @@ -1946,6 +1945,11 @@ public void testIsInValidDestination() throws IOException, Error, XPathExpressio
samlResponse.setDestinationUrl(ACS_URL);
samlResponse.isValid();
assertThat(samlResponse.getError(), not(containsString("The response was received at")));

settings.setWantDestinationUrlValidation(false);
samlResponse = new SamlResponse(settings, newHttpRequest(requestURL, samlResponseEncoded));
samlResponse.isValid();
assertThat(samlResponse.getError(), not(containsString("The response was received at")));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,4 +491,16 @@ public void testValidateMetadataExpired() throws Exception {
assertFalse(errors.isEmpty());
assertTrue(errors.contains("expired_xml"));
}

/**
* Tests the wantDestinationUrlValidation method of Saml2Settings
*/
@Test
public void testIsWantDestinationUrlValidation() {
Saml2Settings settings = new Saml2Settings();

assertTrue(settings.getWantDestinationUrlValidation());
settings.setWantDestinationUrlValidation(false);
assertFalse(settings.getWantDestinationUrlValidation());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,7 @@ public void testLoadFromValues() throws Exception {
assertTrue(setting.getWantAssertionsSigned());
assertTrue(setting.getWantAssertionsEncrypted());
assertTrue(setting.getWantNameIdEncrypted());
assertTrue(setting.getWantDestinationUrlValidation());

List<String> reqAuthContext = new ArrayList<>();
reqAuthContext.add("urn:oasis:names:tc:SAML:2.0:ac:classes:urn:oasis:names:tc:SAML:2.0:ac:classes:Password");
Expand Down Expand Up @@ -925,6 +926,10 @@ public void testLoadFromValues() throws Exception {
assertFalse(previousCert.equals(newCert));
assertFalse(previousKey.equals(newKey));

samlData.put(SECURITY_WANT_DESTINATION_URL_VALIDATION, false);

setting = new SettingsBuilder().fromValues(samlData).build();
assertFalse(setting.getWantDestinationUrlValidation());
}

/**
Expand Down Expand Up @@ -1045,6 +1050,7 @@ public void testLoadFromValuesWithObjects() throws Exception {
assertTrue(setting.getWantAssertionsSigned());
assertTrue(setting.getWantAssertionsEncrypted());
assertTrue(setting.getWantNameIdEncrypted());
assertTrue(setting.getWantDestinationUrlValidation());

List<String> reqAuthContext = new ArrayList<>();
reqAuthContext.add("urn:oasis:names:tc:SAML:2.0:ac:classes:urn:oasis:names:tc:SAML:2.0:ac:classes:Password");
Expand Down