diff --git a/plugins/user-authenticators/saml2/pom.xml b/plugins/user-authenticators/saml2/pom.xml
index 9730a9b5817c..ade87aca704b 100644
--- a/plugins/user-authenticators/saml2/pom.xml
+++ b/plugins/user-authenticators/saml2/pom.xml
@@ -53,5 +53,11 @@
${cs.xercesImpl.version}
test
+
+ org.assertj
+ assertj-core
+ ${cs.assertj.version}
+ test
+
diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
index cbbdbd28bf8a..c65e4c09be6e 100644
--- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
+++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
@@ -150,7 +150,8 @@ public static String buildAuthnRequestUrl(final String authnId, final SAMLProvid
if (spMetadata.getKeyPair() != null) {
privateKey = spMetadata.getKeyPair().getPrivate();
}
- redirectUrl = idpMetadata.getSsoUrl() + "?" + SAMLUtils.generateSAMLRequestSignature("SAMLRequest=" + SAMLUtils.encodeSAMLRequest(authnRequest), privateKey, signatureAlgorithm);
+ String appendOperator = idpMetadata.getSsoUrl().contains("?") ? "&" : "?";
+ redirectUrl = idpMetadata.getSsoUrl() + appendOperator + SAMLUtils.generateSAMLRequestSignature("SAMLRequest=" + SAMLUtils.encodeSAMLRequest(authnRequest), privateKey, signatureAlgorithm);
} catch (ConfigurationException | FactoryConfigurationError | MarshallingException | IOException | NoSuchAlgorithmException | InvalidKeyException | java.security.SignatureException e) {
s_logger.error("SAML AuthnRequest message building error: " + e.getMessage());
}
diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java
index 433fdf3224ad..cf265199b426 100644
--- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java
+++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java
@@ -19,18 +19,22 @@
package org.apache.cloudstack;
-import java.security.KeyPair;
-import java.security.PrivateKey;
-import java.security.PublicKey;
-import java.util.regex.Pattern;
-
+import junit.framework.TestCase;
+import org.apache.cloudstack.saml.SAML2AuthManager;
+import org.apache.cloudstack.saml.SAMLProviderMetadata;
import org.apache.cloudstack.saml.SAMLUtils;
import org.apache.cloudstack.utils.security.CertUtils;
import org.junit.Test;
import org.opensaml.saml2.core.AuthnRequest;
import org.opensaml.saml2.core.LogoutRequest;
-import junit.framework.TestCase;
+import java.net.URI;
+import java.security.KeyPair;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+import java.util.regex.Pattern;
+
+import static org.assertj.core.api.Assertions.assertThat;
public class SAMLUtilsTest extends TestCase {
@@ -60,6 +64,63 @@ public void testBuildAuthnRequestObject() throws Exception {
assertEquals(req.getIssuer().getValue(), spId);
}
+ @Test
+ public void testBuildAuthnRequestUrlWithoutQueryParam() throws Exception {
+ String urlScheme = "http";
+
+ String spDomain = "sp.domain.example";
+ String spUrl = urlScheme + "://" + spDomain;
+ String spId = "serviceProviderId";
+
+ String idpDomain = "idp.domain.example";
+ String idpUrl = urlScheme + "://" + idpDomain;
+ String idpId = "identityProviderId";
+
+ String authnId = SAMLUtils.generateSecureRandomId();
+
+ SAMLProviderMetadata spMetadata = new SAMLProviderMetadata();
+ spMetadata.setEntityId(spId);
+ spMetadata.setSsoUrl(spUrl);
+
+ SAMLProviderMetadata idpMetadata = new SAMLProviderMetadata();
+ idpMetadata.setSsoUrl(idpUrl);
+ idpMetadata.setEntityId(idpId);
+
+ URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value()));
+ assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("SAMLRequest");
+ assertEquals(urlScheme, redirectUrl.getScheme());
+ assertEquals(idpDomain, redirectUrl.getHost());
+ }
+
+ @Test
+ public void testBuildAuthnRequestUrlWithQueryParam() throws Exception {
+ String urlScheme = "http";
+
+ String spDomain = "sp.domain.example";
+ String spUrl = urlScheme + "://" + spDomain;
+ String spId = "cloudstack";
+
+ String idpDomain = "idp.domain.example";
+ String idpQueryParam = "idpid=CX1298373";
+ String idpUrl = urlScheme + "://" + idpDomain + "?" + idpQueryParam;
+ String idpId = "identityProviderId";
+
+ String authnId = SAMLUtils.generateSecureRandomId();
+
+ SAMLProviderMetadata spMetadata = new SAMLProviderMetadata();
+ spMetadata.setEntityId(spId);
+ spMetadata.setSsoUrl(spUrl);
+
+ SAMLProviderMetadata idpMetadata = new SAMLProviderMetadata();
+ idpMetadata.setSsoUrl(idpUrl);
+ idpMetadata.setEntityId(idpId);
+
+ URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value()));
+ assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("idpid").hasParameter("SAMLRequest");
+ assertEquals(urlScheme, redirectUrl.getScheme());
+ assertEquals(idpDomain, redirectUrl.getHost());
+ }
+
@Test
public void testBuildLogoutRequest() throws Exception {
String logoutUrl = "http://logoutUrl";
diff --git a/pom.xml b/pom.xml
index e309e1e02a5e..131c9886d96a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -118,6 +118,7 @@
7.1.0
2.27.2
2.12.2
+ 3.23.1
5.10.0