\KEYCLOAK-5335 Destination attr in SAML requests is optional (#4424)

services/src/main/java/org/keycloak/protocol/saml/SamlService.java

@@ -359,6 +359,11 @@ public class SamlService extends AuthorizationEndpointBase {
         protected Response logoutRequest(LogoutRequestType logoutRequest, ClientModel client, String relayState) {
             SamlClient samlClient = new SamlClient(client);
             // validate destination
+            if (logoutRequest.getDestination() == null && samlClient.requiresClientSignature()) {
+                event.detail(Details.REASON, "invalid_destination");
+                event.error(Errors.INVALID_SAML_LOGOUT_REQUEST);
+                return ErrorPage.error(session, Messages.INVALID_REQUEST);
+            }
             if (! isValidDestination(logoutRequest.getDestination())) {
                 event.detail(Details.REASON, "invalid_destination");
                 event.error(Errors.INVALID_SAML_LOGOUT_REQUEST);
@@ -710,7 +715,7 @@ public class SamlService extends AuthorizationEndpointBase {
 
     private boolean isValidDestination(URI destination) {
         if (destination == null) {
-            return false;
+            return true;    // destination is optional
         }
 
         URI expected = uriInfo.getAbsolutePath();

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java

@@ -280,7 +280,8 @@ public class SamlClient {
     public static AuthnRequestType createLoginRequestDocument(String issuer, String assertionConsumerURL, URI destination) {
         try {
             SAML2Request samlReq = new SAML2Request();
-            AuthnRequestType loginReq = samlReq.createAuthnRequestType(UUID.randomUUID().toString(), assertionConsumerURL, destination.toString(), issuer);
+            AuthnRequestType loginReq = samlReq.createAuthnRequestType(UUID.randomUUID().toString(), assertionConsumerURL,
+              destination == null ? null : destination.toString(), issuer);
 
             return loginReq;
         } catch (ConfigurationException ex) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java

@@ -29,6 +29,11 @@ public abstract class AbstractSamlTest extends AbstractAuthTest {
     protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST2 = "http://localhost:8080/sales-post2/";
     protected static final String SAML_CLIENT_ID_SALES_POST2 = "http://localhost:8081/sales-post2/";
 
+    protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG = "http://localhost:8081/sales-post-sig/";
+    protected static final String SAML_CLIENT_ID_SALES_POST_SIG = "http://localhost:8081/sales-post-sig/";
+    protected static final String SAML_CLIENT_ID_SALES_POST_SIG_PRIVATE_KEY = "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAL72RWA8WmyGNLZpPaDnaWpO/aWpvWOC9dlWTuIUXgsA7pJ0JcpdFkjhK7MhGRU2kTVrjZXWRBGVqjOgWSWC9fmyN7y2Slr3KWr5vEZcz1FHJypG7YH6hZRXsCqrsGzRe53RZ4dBhFwSgKOqwUobLrmqQtEN1PvQVMmClIe2z6APAgMBAAECgYA1TV5+BzqiMi/Cfsux/wYAo33PYPq5LRPcj2fDWTYK0j7FaGAoBSW0QA3HmUR8FFgh1hyWJ1GmquTwNiDMBKsNhMdfOpUEl8CTVWsAamrJGr5M0wtQeReJOJIM5DpaqFf3dQYyYCBCbVGwtrr5/LUE78HbLzn5h/Y5TKWwpLpq8QJBAOyVEOI0V20SN2/I4vbS1Dv5b54pXFXEoDJDeAWMggOLMQtm34mrVbPpdX02ErUkW2VW6B4PW2Vb/4rXp8SYNhcCQQDOoqg38l4ZvbSt9CV0BRZ1BtvSOtABiWChJT/19gf1hUlPy+eR+E8FPAjD0f165/X/+VE/+MeW0d3lyHjbfRjJAkEA3es6SiW0+IAE9lum4saDBLsHA4JitaVaa6u0EuhpMK/JUpuuBfJs0vWkGs61H6u5+8ZYt5HKNrrkazW9joEFAwJBAJOw2r8yMmP/nbZ/vI1SXZzDjDaU5rtSb4h+UVsBwOqRm7a3LQq+CezZ3gHog15nkQKmNpacwDtiQVHNmeR3Y1ECQHyURjnpJyc2KChmSSO0RR3Z6N2Fk9i5L1Ip+/3MfA34j4MTa9UVnPwY+r+ItwzHXc+Rlphok1DgW1Bj+FEtKfk=";
+    protected static final String SAML_CLIENT_ID_SALES_POST_SIG_PUBLIC_KEY = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC+9kVgPFpshjS2aT2g52lqTv2lqb1jgvXZVk7iFF4LAO6SdCXKXRZI4SuzIRkVNpE1a42V1kQRlaozoFklgvX5sje8tkpa9ylq+bxGXM9RRycqRu2B+oWUV7Aqq7Bs0Xud0WeHQYRcEoCjqsFKGy65qkLRDdT70FTJgpSHts+gDwIDAQAB";
+
     protected static final String SAML_ASSERTION_CONSUMER_URL_SALES_POST_ENC = "http://localhost:8080/sales-post-enc/";
     protected static final String SAML_CLIENT_ID_SALES_POST_ENC = "http://localhost:8081/sales-post-enc/";
     protected static final String SAML_CLIENT_SALES_POST_ENC_PRIVATE_KEY = "MIICXQIBAAKBgQDb7kwJPkGdU34hicplwfp6/WmNcaLh94TSc7Jyr9Undp5pkyLgb0DE7EIE+6kSs4LsqCb8HDkB0nLD5DXbBJFd8n0WGoKstelvtg6FtVJMnwN7k7yZbfkPECWH9zF70VeOo9vbzrApNRnct8ZhH5fbflRB4JMA9L9R+LbURdoSKQIDAQABAoGBANtbZG9bruoSGp2s5zhzLzd4hczT6Jfk3o9hYjzNb5Z60ymN3Z1omXtQAdEiiNHkRdNxK+EM7TcKBfmoJqcaeTkW8cksVEAW23ip8W9/XsLqmbU2mRrJiKa+KQNDSHqJi1VGyimi4DDApcaqRZcaKDFXg2KDr/Qt5JFD/o9IIIPZAkEA+ZENdBIlpbUfkJh6Ln+bUTss/FZ1FsrcPZWu13rChRMrsmXsfzu9kZUWdUeQ2Dj5AoW2Q7L/cqdGXS7Mm5XhcwJBAOGZq9axJY5YhKrsksvYRLhQbStmGu5LG75suF+rc/44sFq+aQM7+oeRr4VY88Mvz7mk4esdfnk7ae+cCazqJvMCQQCx1L1cZw3yfRSn6S6u8XjQMjWE/WpjulujeoRiwPPY9WcesOgLZZtYIH8nRL6ehEJTnMnahbLmlPFbttxPRUanAkA11MtSIVcKzkhp2KV2ipZrPJWwI18NuVJXb+3WtjypTrGWFZVNNkSjkLnHIeCYlJIGhDd8OL9zAiBXEm6kmgLNAkBWAg0tK2hCjvzsaA505gWQb4X56uKWdb0IzN+fOLB3Qt7+fLqbVQNQoNGzqey6B4MoS1fUKAStqdGTFYPG/+9t";

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BasicSamlTest.java

@@ -52,6 +52,47 @@ public class BasicSamlTest extends AbstractSamlTest {
         assertThat(documentToString(document.getSamlDocument()), not(containsString("InResponseTo=\"" + System.getProperty("java.version") + "\"")));
     }
 
+    @Test
+    public void testNoDestinationPost() throws Exception {
+        AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
+
+        Document doc = SAML2Request.convert(loginRep);
+        HttpUriRequest post = Binding.POST.createSamlUnsignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc);
+
+        try (CloseableHttpClient client = HttpClientBuilder.create().setRedirectStrategy(new RedirectStrategyWithSwitchableFollowRedirect()).build();
+          CloseableHttpResponse response = client.execute(post)) {
+            assertThat(response, statusCodeIsHC(Response.Status.OK));
+            assertThat(EntityUtils.toString(response.getEntity(), "UTF-8"), containsString("login"));
+        }
+    }
+
+    @Test
+    public void testNoDestinationRedirect() throws Exception {
+        AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
+
+        Document doc = SAML2Request.convert(loginRep);
+        HttpUriRequest post = Binding.REDIRECT.createSamlUnsignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc);
+
+        try (CloseableHttpClient client = HttpClientBuilder.create().setRedirectStrategy(new RedirectStrategyWithSwitchableFollowRedirect()).build();
+          CloseableHttpResponse response = client.execute(post)) {
+            assertThat(response, statusCodeIsHC(Response.Status.OK));
+            assertThat(EntityUtils.toString(response.getEntity(), "UTF-8"), containsString("login"));
+        }
+    }
+
+    @Test
+    public void testNoDestinationSignedPost() throws Exception {
+        AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(SAML_CLIENT_ID_SALES_POST_SIG, SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, null);
+
+        Document doc = SAML2Request.convert(loginRep);
+        HttpUriRequest post = Binding.POST.createSamlSignedRequest(getAuthServerSamlEndpoint(REALM_NAME), null, doc, SAML_CLIENT_ID_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_ID_SALES_POST_SIG_PUBLIC_KEY);
+
+        try (CloseableHttpClient client = HttpClientBuilder.create().setRedirectStrategy(new RedirectStrategyWithSwitchableFollowRedirect()).build();
+          CloseableHttpResponse response = client.execute(post)) {
+            assertThat(response, statusCodeIsHC(Response.Status.INTERNAL_SERVER_ERROR));
+        }
+    }
+
     @Test
     public void testNoPortInDestination() throws Exception {
         // note that this test relies on settings of the login-protocol.saml.knownProtocols configuration option