mirror of https://github.com/apache/cloudstack.git
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 <rohit.yadav@shapeblue.com>
(cherry picked from commit b2b496288d)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This commit is contained in:
parent
1a8fe82580
commit
0b94f254e8
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Boolean, ActionOnFailedAuthentication> pair1 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, null);
|
||||
Assert.assertFalse(pair1.first());
|
||||
|
||||
// When there is SAMLRequest in params
|
||||
Pair<Boolean, ActionOnFailedAuthentication> pair;
|
||||
Map<String, Object[]> params = new HashMap<String, Object[]>();
|
||||
|
||||
// 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<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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue