FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

CLOUDSTACK-8034: Hash user IDs for SAML authentication · kwanggithub/cloudstack@0b94f25 · GitHub

This repository was archived by the owner on Jan 15, 2020. It is now read-only.
/ cloudstack Public archive
forked from apache/cloudstack

Commit 0b94f25

Browse files
yadvr
committed
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 b2b4962) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
1 parent 1a8fe82 commit 0b94f25

4 files changed

Lines changed: 42 additions & 14 deletions

File tree

‎plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2UserAuthenticator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username,
4848
return new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
4949
} else {
5050
User user = _userDao.getUser(userAccount.getId());
51-
if (user != null && SAMLUtils.checkSAMLUserId(user.getUuid()) &&
51+
if (user != null && SAMLUtils.checkSAMLUser(user.getUuid(), username) &&
5252
requestParameters != null && requestParameters.containsKey(SAMLUtils.SAML_RESPONSE)) {
5353
return new Pair<Boolean, ActionOnFailedAuthentication>(true, null);
5454
}

‎plugins/user-authenticators/saml2/test/org/apache/cloudstack/SAML2UserAuthenticatorTest.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,28 @@ public void authenticate() throws NoSuchFieldException, SecurityException, Illeg
7373
Mockito.when(userAccountDao.getUserAccount(Mockito.anyString(), Mockito.anyLong())).thenReturn(account);
7474
Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
7575

76+
Pair<Boolean, ActionOnFailedAuthentication> pair;
77+
Map<String, Object[]> params = new HashMap<String, Object[]>();
78+
7679
// When there is no SAMLRequest in params
77-
Pair<Boolean, ActionOnFailedAuthentication> pair1 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, null);
78-
Assert.assertFalse(pair1.first());
80+
pair = authenticator.authenticate("someUID", "random", 1l, params);
81+
Assert.assertFalse(pair.first());
7982

80-
// When there is SAMLRequest in params
81-
Map<String, Object[]> params = new HashMap<String, Object[]>();
83+
// When there is SAMLRequest in params and user is same as the mocked one
8284
params.put(SAMLUtils.SAML_RESPONSE, new Object[]{});
83-
Pair<Boolean, ActionOnFailedAuthentication> pair2 = authenticator.authenticate(SAMLUtils.createSAMLId("user1234"), "random", 1l, params);
84-
Assert.assertTrue(pair2.first());
85+
pair = authenticator.authenticate("someUID", "random", 1l, params);
86+
Assert.assertTrue(pair.first());
87+
88+
// When there is SAMLRequest in params but username is null
89+
pair = authenticator.authenticate(null, "random", 1l, params);
90+
Assert.assertFalse(pair.first());
91+
92+
// When there is SAMLRequest in params but username is empty
93+
pair = authenticator.authenticate("", "random", 1l, params);
94+
Assert.assertFalse(pair.first());
95+
96+
// When there is SAMLRequest in params but username is not valid
97+
pair = authenticator.authenticate("someOtherUID", "random", 1l, params);
98+
Assert.assertFalse(pair.first());
8599
}
86100
}

‎utils/src/org/apache/cloudstack/utils/auth/SAMLUtils.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.apache.cloudstack.utils.auth;
2121

2222
import com.cloud.utils.HttpUtils;
23+
import org.apache.commons.codec.digest.DigestUtils;
2324
import org.apache.log4j.Logger;
2425
import org.bouncycastle.jce.provider.BouncyCastleProvider;
2526
import org.bouncycastle.x509.X509V1CertificateGenerator;
@@ -96,18 +97,25 @@ public class SAMLUtils {
9697
public static final Logger s_logger = Logger.getLogger(SAMLUtils.class);
9798

9899
public static final String SAML_RESPONSE = "SAMLResponse";
99-
public static final String SAML_NS = "saml://";
100+
public static final String SAML_NS = "SAML-";
100101
public static final String SAML_NAMEID = "SAML_NAMEID";
101102
public static final String SAML_SESSION = "SAML_SESSION";
102103
public static final String CERTIFICATE_NAME = "SAMLSP_CERTIFICATE";
103104

104105
public static String createSAMLId(String uid) {
105-
String samlUuid = SAML_NS + uid;
106-
return samlUuid.length() > 40 ? samlUuid.substring(0, 40) : samlUuid;
106+
if (uid == null) {
107+
return null;
108+
}
109+
String hash = DigestUtils.sha256Hex(uid);
110+
String samlUuid = SAML_NS + hash;
111+
return samlUuid.substring(0, 40);
107112
}
108113

109-
public static Boolean checkSAMLUserId(String uuid) {
110-
return uuid.startsWith(SAML_NS);
114+
public static boolean checkSAMLUser(String uuid, String username) {
115+
if (uuid == null || uuid.isEmpty() || username == null || username.isEmpty()) {
116+
return false;
117+
}
118+
return uuid.startsWith(SAML_NS) && createSAMLId(username).equals(uuid);
111119
}
112120

113121
public static String generateSecureRandomId() {

‎utils/test/org/apache/cloudstack/utils/auth/SAMLUtilsTest.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,14 @@ public class SAMLUtilsTest extends TestCase {
3434

3535
@Test
3636
public void testSAMLId() throws Exception {
37-
assertTrue(SAMLUtils.checkSAMLUserId(SAMLUtils.createSAMLId("someUID")));
38-
assertFalse(SAMLUtils.checkSAMLUserId("randomUID"));
37+
assertEquals(SAMLUtils.createSAMLId(null), null);
38+
assertEquals(SAMLUtils.createSAMLId("someUserName"), "SAML-305e19dd2581f33fd90b3949298ec8b17de");
39+
40+
assertTrue(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId("someUserName"), "someUserName"));
41+
assertFalse(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId("someUserName"), "someOtherUserName"));
42+
assertFalse(SAMLUtils.checkSAMLUser(SAMLUtils.createSAMLId(null), "someOtherUserName"));
43+
assertFalse(SAMLUtils.checkSAMLUser("randomUID", "randomUID"));
44+
assertFalse(SAMLUtils.checkSAMLUser(null, null));
3945
}
4046

4147
@Test

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL