diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java index e623fc21798..31a93a43780 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java @@ -48,7 +48,7 @@ public class SAML2UserAuthenticator extends DefaultUserAuthenticator { return new Pair(false, null); } else { User user = _userDao.getUser(userAccount.getId()); - if (user != null && SAMLUtils.checkSAMLUserId(user.getUuid()) && + if (user != null && SAMLUtils.checkSAMLUser(user.getUuid(), username) && requestParameters != null && requestParameters.containsKey(SAMLUtils.SAML_RESPONSE)) { return new Pair(true, null); } diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java index 29fb4969e83..83792c64d0f 100644 --- a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java @@ -73,14 +73,28 @@ public class SAML2UserAuthenticatorTest { Mockito.when(userAccountDao.getUserAccount(Mockito.anyString(), Mockito.anyLong())).thenReturn(account); Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user); - // When there is no SAMLRequest in params - Pair pair1 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, null); - Assert.assertFalse(pair1.first()); - - // When there is SAMLRequest in params + Pair pair; Map params = new HashMap(); + + // When there is no SAMLRequest in params + pair = authenticator.authenticate("someUID", "random", 1l, params); + Assert.assertFalse(pair.first()); + + // When there is SAMLRequest in params and user is same as the mocked one params.put(SAMLUtils.SAML_RESPONSE, new Object[]{}); - Pair pair2 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, params); - Assert.assertTrue(pair2.first()); + pair = authenticator.authenticate("someUID", "random", 1l, params); + Assert.assertTrue(pair.first()); + + // When there is SAMLRequest in params but username is null + pair = authenticator.authenticate(null, "random", 1l, params); + Assert.assertFalse(pair.first()); + + // When there is SAMLRequest in params but username is empty + pair = authenticator.authenticate("", "random", 1l, params); + Assert.assertFalse(pair.first()); + + // When there is SAMLRequest in params but username is not valid + pair = authenticator.authenticate("someOtherUID", "random", 1l, params); + Assert.assertFalse(pair.first()); } } diff --git a/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java b/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java index d129309ba98..dbd2d6fac52 100644 --- a/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java +++ b/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.utils.auth; import com.cloud.utils.HttpUtils; +import org.apache.commons.codec.digest.DigestUtils; import org.apache.log4j.Logger; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.x509.X509V1CertificateGenerator; @@ -96,18 +97,25 @@ public class SAMLUtils { public static final Logger s_logger = Logger.getLogger(SAMLUtils.class); public static final String SAML_RESPONSE = "SAMLResponse"; - public static final String SAML_NS = "saml://"; + public static final String SAML_NS = "SAML-"; public static final String SAML_NAMEID = "SAML_NAMEID"; public static final String SAML_SESSION = "SAML_SESSION"; public static final String CERTIFICATE_NAME = "SAMLSP_CERTIFICATE"; public static String createSAMLId(String uid) { - String samlUuid = SAML_NS + uid; - return samlUuid.length() > 40 ? samlUuid.substring(0, 40) : samlUuid; + if (uid == null) { + return null; + } + String hash = DigestUtils.sha256Hex(uid); + String samlUuid = SAML_NS + hash; + return samlUuid.substring(0, 40); } - public static Boolean checkSAMLUserId(String uuid) { - return uuid.startsWith(SAML_NS); + public static boolean checkSAMLUser(String uuid, String username) { + if (uuid == null || uuid.isEmpty() || username == null || username.isEmpty()) { + return false; + } + return uuid.startsWith(SAML_NS) && createSAMLId(username).equals(uuid); } public static String generateSecureRandomId() { diff --git a/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java b/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java index 85be2ef9ad0..bebfd130441 100644 --- a/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java +++ b/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java @@ -34,8 +34,14 @@ public class SAMLUtilsTest extends TestCase { @Test public void testSAMLId() throws Exception { - assertTrue(SAMLUtils.checkSAMLUserId(SAMLUtils.createSAMLId("someUID"))); - assertFalse(SAMLUtils.checkSAMLUserId("randomUID")); + assertEquals(SAMLUtils.createSAMLId(null), null); + assertEquals(SAMLUtils.createSAMLId("someUserName"), "SAML-305e19dd2581f33fd90b3949298ec8b17de"); + + assertTrue(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId("someUserName"), "someUserName")); + assertFalse(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId("someUserName"), "someOtherUserName")); + assertFalse(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId(null), "someOtherUserName")); + assertFalse(SAMLUtils.checkSAMLUser("randomUID", "randomUID")); + assertFalse(SAMLUtils.checkSAMLUser(null, null)); } @Test