From 42d0418530599717624f4614220d06c83ea48c2a Mon Sep 17 00:00:00 2001 From: Miguel Ferreira Date: Wed, 23 Dec 2015 14:49:29 +0100 Subject: [PATCH 1/3] Code formatting --- .../cloudstack/ldap/LdapAuthenticator.java | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java index 792129247fc2..87558d1e6d71 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -54,8 +54,8 @@ public LdapAuthenticator(final LdapManager ldapManager, final UserAccountDao use } @Override - public Pair authenticate(final String username, final String password, final Long domainId, final Map requestParameters) { - + public Pair authenticate(final String username, final String password, final Long domainId, + final Map requestParameters) { if (StringUtils.isEmpty(username) || StringUtils.isEmpty(password)) { s_logger.debug("Username or Password cannot be empty"); return new Pair(false, null); @@ -66,14 +66,14 @@ public Pair authenticate(final String use if (_ldapManager.isLdapEnabled()) { final UserAccount user = _userAccountDao.getUserAccount(username, domainId); - LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); - if(ldapTrustMapVO != null) { + final LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); + if (ldapTrustMapVO != null) { try { - LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType().toString(), ldapTrustMapVO.getName()); - if(!ldapUser.isDisabled()) { + final LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType().toString(), ldapTrustMapVO.getName()); + if (!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); - if(result) { - if(user == null) { + if (result) { + if (user == null) { // import user to cloudstack createCloudStackUserAccount(ldapUser, domainId, ldapTrustMapVO.getAccountType()); } else { @@ -81,24 +81,24 @@ public Pair authenticate(final String use } } } else { - //disable user in cloudstack + // disable user in cloudstack disableUserInCloudStack(user); } - } catch (NoLdapUserMatchingQueryException e) { + } catch (final NoLdapUserMatchingQueryException e) { s_logger.debug(e.getMessage()); } } else { - //domain is not linked to ldap follow normal authentication - if(user != null ) { + // domain is not linked to ldap follow normal authentication + if (user != null) { try { - LdapUser ldapUser = _ldapManager.getUser(username); - if(!ldapUser.isDisabled()) { + final LdapUser ldapUser = _ldapManager.getUser(username); + if (!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); } else { - s_logger.debug("user with principal "+ ldapUser.getPrincipal() + " is disabled in ldap"); + s_logger.debug("user with principal " + ldapUser.getPrincipal() + " is disabled in ldap"); } - } catch (NoLdapUserMatchingQueryException e) { + } catch (final NoLdapUserMatchingQueryException e) { s_logger.debug(e.getMessage()); } } @@ -111,19 +111,19 @@ public Pair authenticate(final String use return new Pair(result, action); } - private void enableUserInCloudStack(UserAccount user) { - if(user != null && (user.getState().equalsIgnoreCase(Account.State.disabled.toString()))) { + private void enableUserInCloudStack(final UserAccount user) { + if (user != null && user.getState().equalsIgnoreCase(Account.State.disabled.toString())) { _accountManager.enableUser(user.getId()); } } - private void createCloudStackUserAccount(LdapUser user, long domainId, short accountType) { - String username = user.getUsername(); - _accountManager.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, username, accountType, domainId, username, null, - UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); + private void createCloudStackUserAccount(final LdapUser user, final long domainId, final short accountType) { + final String username = user.getUsername(); + _accountManager.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, ldapGroupName, accountType, domainId, + username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); } - private void disableUserInCloudStack(UserAccount user) { + private void disableUserInCloudStack(final UserAccount user) { if (user != null) { _accountManager.disableUser(user.getId()); } From 0089647c11a36e23cb89770f63c971ee8c12fe89 Mon Sep 17 00:00:00 2001 From: Miguel Ferreira Date: Wed, 23 Dec 2015 15:16:48 +0100 Subject: [PATCH 2/3] Enable Java based unit tests in mvn lifecycle --- plugins/user-authenticators/ldap/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/user-authenticators/ldap/pom.xml b/plugins/user-authenticators/ldap/pom.xml index 04c212e5a4bf..c994a5e085fb 100644 --- a/plugins/user-authenticators/ldap/pom.xml +++ b/plugins/user-authenticators/ldap/pom.xml @@ -71,6 +71,7 @@ **/*Spec* + **/*Test.java From 1ca4e484e0730f7a114dd4af7abcb2bcc11e7337 Mon Sep 17 00:00:00 2001 From: Miguel Ferreira Date: Wed, 23 Dec 2015 14:49:53 +0100 Subject: [PATCH 3/3] Make ACS account using LDAP group name --- .../ldap/DistinguishedNameParser.java | 27 ++++++++++++ .../cloudstack/ldap/LdapAuthenticator.java | 15 ++++++- .../ldap/DistinguishedNameParserTest.java | 43 +++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/DistinguishedNameParser.java create mode 100644 plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/DistinguishedNameParserTest.java diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/DistinguishedNameParser.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/DistinguishedNameParser.java new file mode 100644 index 000000000000..0ea49df65c88 --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/DistinguishedNameParser.java @@ -0,0 +1,27 @@ +package org.apache.cloudstack.ldap; + +public class DistinguishedNameParser { + + private static final String KEY_VALUE_SEPARATOR = "="; + private static final String NAME_SEPARATOR = ","; + + public static String parseLeafName(final String distinguishedName) { + if (distinguishedName.contains(NAME_SEPARATOR)) { + final String[] parts = distinguishedName.split(NAME_SEPARATOR); + return parseValue(parts[0]); + } else { + return parseValue(distinguishedName); + } + } + + private static String parseValue(final String distinguishedName) { + if (distinguishedName.contains(KEY_VALUE_SEPARATOR)) { + final String[] parts = distinguishedName.split(KEY_VALUE_SEPARATOR); + if (parts.length > 1) { + return parts[1]; + } + } + throw new IllegalArgumentException("Malformed distinguished name: " + distinguishedName); + } + +} diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java index 87558d1e6d71..ed027b9922e1 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -43,6 +43,8 @@ public class LdapAuthenticator extends AdapterBase implements UserAuthenticator @Inject private AccountManager _accountManager; + private String ldapGroupName; + public LdapAuthenticator() { super(); } @@ -68,6 +70,7 @@ public Pair authenticate(final String use final UserAccount user = _userAccountDao.getUserAccount(username, domainId); final LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); if (ldapTrustMapVO != null) { + ldapGroupName = DistinguishedNameParser.parseLeafName(ldapTrustMapVO.getName()); try { final LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType().toString(), ldapTrustMapVO.getName()); if (!ldapUser.isDisabled()) { @@ -119,8 +122,16 @@ private void enableUserInCloudStack(final UserAccount user) { private void createCloudStackUserAccount(final LdapUser user, final long domainId, final short accountType) { final String username = user.getUsername(); - _accountManager.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, ldapGroupName, accountType, domainId, - username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); + final Account account = _accountManager.getActiveAccountByName(ldapGroupName, domainId); + if (account == null) { + s_logger.info("Account (" + ldapGroupName + ") for LDAP group does not exist. Creating account and user (" + username + ")."); + _accountManager.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, ldapGroupName, accountType, domainId, + username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); + } else { + s_logger.debug("Account (" + ldapGroupName + ") for LDAP group already exists not exist. Creating user (" + username + ")."); + _accountManager.createUser(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, ldapGroupName, domainId, + UUID.randomUUID().toString(), User.Source.LDAP); + } } private void disableUserInCloudStack(final UserAccount user) { diff --git a/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/DistinguishedNameParserTest.java b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/DistinguishedNameParserTest.java new file mode 100644 index 000000000000..d2e892fd21c7 --- /dev/null +++ b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/DistinguishedNameParserTest.java @@ -0,0 +1,43 @@ +package org.apache.cloudstack.ldap; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +import org.junit.Test; + +public class DistinguishedNameParserTest { + + @Test(expected = IllegalArgumentException.class) + public void testParseLeafeNameEmptyString() throws Exception { + final String distinguishedName = ""; + DistinguishedNameParser.parseLeafName(distinguishedName); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseLeafeNameSingleMalformedKeyValue() throws Exception { + final String distinguishedName = "someMalformedKeyValue"; + DistinguishedNameParser.parseLeafName(distinguishedName); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseLeafeNameNonSingleMalformedKeyValue() throws Exception { + final String distinguishedName = "someMalformedKeyValue,key=value"; + DistinguishedNameParser.parseLeafName(distinguishedName); + } + + @Test + public void testParseLeafeNameSingleKeyValue() throws Exception { + final String distinguishedName = "key=value"; + final String value = DistinguishedNameParser.parseLeafName(distinguishedName); + + assertThat(value, equalTo("value")); + } + + @Test + public void testParseLeafeNameMultipleKeyValue() throws Exception { + final String distinguishedName = "key1=leaf,key2=nonleaf"; + final String value = DistinguishedNameParser.parseLeafName(distinguishedName); + + assertThat(value, equalTo("leaf")); + } +}