Repository: cloudstack Updated Branches: refs/heads/4.5 1e8476d38 -> b2b496288
CLOUDSTACK-8034: Hash user IDs for SAML authentication The User table's UUID column is restricted to 40 chars only, since we don't know how long the nameID/userID of a SAML authenticated user will be - the fix hashes that user ID and takes a substring of length 40 chars. For hashing, SHA256 is used which returns a 64 char length string. - Fix tests, add test cases - Improve checkSAMLUser method - Use SHA256 one way hashing to create unique UUID for SAML users Signed-off-by: Rohit Yadav <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/b2b49628 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/b2b49628 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/b2b49628 Branch: refs/heads/4.5 Commit: b2b496288d164fead2c089fb48319e1988b03ce8 Parents: 1e8476d Author: Rohit Yadav <[email protected]> Authored: Mon Jan 12 13:33:57 2015 +0530 Committer: Rohit Yadav <[email protected]> Committed: Mon Jan 12 13:33:57 2015 +0530 ---------------------------------------------------------------------- .../cloudstack/saml/SAML2UserAuthenticator.java | 2 +- .../cloudstack/SAML2UserAuthenticatorTest.java | 26 +++++++++++++++----- .../apache/cloudstack/utils/auth/SAMLUtils.java | 18 ++++++++++---- .../cloudstack/utils/auth/SAMLUtilsTest.java | 10 ++++++-- 4 files changed, 42 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b2b49628/plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java ---------------------------------------------------------------------- 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 e623fc2..31a93a4 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<Boolean, ActionOnFailedAuthentication>(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<Boolean, ActionOnFailedAuthentication>(true, null); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b2b49628/plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java ---------------------------------------------------------------------- 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 29fb496..83792c6 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); + Pair<Boolean, ActionOnFailedAuthentication> pair; + Map<String, Object[]> params = new HashMap<String, Object[]>(); + // When there is no SAMLRequest in params - Pair<Boolean, ActionOnFailedAuthentication> pair1 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, null); - Assert.assertFalse(pair1.first()); + pair = authenticator.authenticate("someUID", "random", 1l, params); + Assert.assertFalse(pair.first()); - // When there is SAMLRequest in params - Map<String, Object[]> params = new HashMap<String, Object[]>(); + // When there is SAMLRequest in params and user is same as the mocked one params.put(SAMLUtils.SAML_RESPONSE, new Object[]{}); - Pair<Boolean, ActionOnFailedAuthentication> 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()); } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b2b49628/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java ---------------------------------------------------------------------- diff --git a/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java b/utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java index d129309..dbd2d6f 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() { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b2b49628/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java ---------------------------------------------------------------------- diff --git a/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java b/utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java index 85be2ef..bebfd13 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
