From c8c76cc03fba2084cd7993ff0506020e8bb6613d Mon Sep 17 00:00:00 2001 From: Ola Bergefall Date: Thu, 7 Jun 2018 16:00:49 +1000 Subject: [PATCH] KEYCLOAK-7316: Default back to false if isPassive is missing in request. --- .../core/parsers/saml/SAMLParserTest.java | 25 ++++++++++++++++++- .../saml/KEYCLOAK-7316-noAtrributes.xml | 14 +++++++++++ .../KEYCLOAK-7316-withFalseAttributes.xml | 16 ++++++++++++ .../saml/KEYCLOAK-7316-withTrueAttributes.xml | 16 ++++++++++++ .../keycloak/protocol/saml/SamlService.java | 6 +++-- .../testsuite/saml/AuthnRequestTest.java | 17 ++++++++++++- 6 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-noAtrributes.xml create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withFalseAttributes.xml create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withTrueAttributes.xml diff --git a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java index 00e0bb37c5..c54bbeaa2d 100644 --- a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java +++ b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java @@ -96,12 +96,13 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; - +import static org.junit.Assert.assertFalse; /** * Test class for SAML parser. * @@ -680,6 +681,28 @@ public class SAMLParserTest { assertThat(req.getRequestedAuthnContext().getAuthnContextDeclRef(), hasItem(is("urn:kc:SAML:2.0:ac:ref:demo:decl"))); } + @Test //https://issues.jboss.org/browse/KEYCLOAK-7316 + public void testAuthnRequestOptionalIsPassive() throws Exception { + AuthnRequestType req = assertParsed("KEYCLOAK-7316-noAtrributes.xml", AuthnRequestType.class); + + assertThat("Not null!", req.isIsPassive(), nullValue()); + assertThat("Not null!", req.isForceAuthn(), nullValue()); + + req = assertParsed("KEYCLOAK-7316-withTrueAttributes.xml", AuthnRequestType.class); + + assertThat(req.isIsPassive(), notNullValue()); + assertTrue("Wrong value!", req.isIsPassive().booleanValue()); + assertThat(req.isForceAuthn(), notNullValue()); + assertTrue("Wrong value!", req.isForceAuthn().booleanValue()); + + req = assertParsed("KEYCLOAK-7316-withFalseAttributes.xml", AuthnRequestType.class); + + assertThat(req.isIsPassive(), notNullValue()); + assertFalse("Wrong value!", req.isIsPassive().booleanValue()); + assertThat(req.isForceAuthn(), notNullValue()); + assertFalse("Wrong value!", req.isForceAuthn().booleanValue()); + } + @Test public void testAuthnRequestInvalidPerXsdWithValidationDisabled() throws Exception { AuthnRequestType req = assertParsed("saml20-authnrequest-invalid-per-xsd.xml", AuthnRequestType.class); diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-noAtrributes.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-noAtrributes.xml new file mode 100644 index 0000000000..43d5998c7b --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-noAtrributes.xml @@ -0,0 +1,14 @@ + + https://iif.example.com/idp/module.php/saml/sp/metadata.php/default-sp + + https://some.domain/sp + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withFalseAttributes.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withFalseAttributes.xml new file mode 100644 index 0000000000..149e1685be --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withFalseAttributes.xml @@ -0,0 +1,16 @@ + + https://iif.example.com/idp/module.php/saml/sp/metadata.php/default-sp + + https://some.domain/sp + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withTrueAttributes.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withTrueAttributes.xml new file mode 100644 index 0000000000..b964cb4d1a --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-7316-withTrueAttributes.xml @@ -0,0 +1,16 @@ + + https://iif.example.com/idp/module.php/saml/sp/metadata.php/default-sp + + https://some.domain/sp + + diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java index 89628f9f28..e6f1833b44 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -338,8 +338,10 @@ public class SamlService extends AuthorizationEndpointBase { } } - - return newBrowserAuthentication(authSession, requestAbstractType.isIsPassive(), redirectToAuthentication); + //If unset we fall back to default "false" + final boolean isPassive = (null == requestAbstractType.isIsPassive() ? + false : requestAbstractType.isIsPassive().booleanValue()); + return newBrowserAuthentication(authSession, isPassive, redirectToAuthentication); } protected String getBindingType(AuthnRequestType requestAbstractType) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AuthnRequestTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AuthnRequestTest.java index 7452ecbedf..3b02ea2901 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AuthnRequestTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AuthnRequestTest.java @@ -33,7 +33,6 @@ public class AuthnRequestTest extends AbstractSamlTest { // KEYCLOAK-7316 @Test - @Ignore public void testIsPassiveNotSet() throws Exception { String res = new SamlClientBuilder() .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, Binding.POST) @@ -48,6 +47,22 @@ public class AuthnRequestTest extends AbstractSamlTest { assertThat(res, containsString("login")); } + // KEYCLOAK-7316 + @Test + public void testIsForceAuthNotSet() throws Exception { + String res = new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, Binding.POST) + .transformObject(so -> { + so.setForceAuthn(null); + return so; + }) + .build() + + .executeAndTransform(resp -> EntityUtils.toString(resp.getEntity())); + + assertThat(res, containsString("login")); + } + // KEYCLOAK-7316 @Test public void testIsPassiveFalse() throws Exception {