[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/1285#issuecomment-202347507
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 139
 Hypervisor xenserver
 NetworkType Advanced
 Passed=104
 Failed=1
 Skipped=4

_Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0


**Failed tests:**
* integration.smoke.test_volumes.TestCreateVolume

 * test_02_attach_volume Failed


**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_deploy_vgpu_enabled_vm
test_06_copy_template
test_06_copy_iso

**Passed test suits:**
integration.smoke.test_deploy_vm_with_userdata.TestDeployVmWithUserData

integration.smoke.test_affinity_groups_projects.TestDeployVmWithAffinityGroup
integration.smoke.test_portable_publicip.TestPortablePublicIPAcquire
integration.smoke.test_over_provisioning.TestUpdateOverProvision
integration.smoke.test_global_settings.TestUpdateConfigWithScope
integration.smoke.test_scale_vm.TestScaleVm
integration.smoke.test_service_offerings.TestCreateServiceOffering
integration.smoke.test_loadbalance.TestLoadBalance
integration.smoke.test_routers.TestRouterServices
integration.smoke.test_reset_vm_on_reboot.TestResetVmOnReboot
integration.smoke.test_snapshots.TestSnapshotRootDisk

integration.smoke.test_deploy_vms_with_varied_deploymentplanners.TestDeployVmWithVariedPlanners
integration.smoke.test_network.TestDeleteAccount
integration.smoke.test_non_contigiousvlan.TestUpdatePhysicalNetwork
integration.smoke.test_deploy_vm_iso.TestDeployVMFromISO
integration.smoke.test_public_ip_range.TestDedicatePublicIPRange
integration.smoke.test_multipleips_per_nic.TestDeployVM
integration.smoke.test_regions.TestRegions
integration.smoke.test_affinity_groups.TestDeployVmWithAffinityGroup
integration.smoke.test_network_acl.TestNetworkACL
integration.smoke.test_pvlan.TestPVLAN
integration.smoke.test_ssvm.TestSSVMs
integration.smoke.test_nic.TestNic
integration.smoke.test_deploy_vm_root_resize.TestDeployVM
integration.smoke.test_resource_detail.TestResourceDetail
integration.smoke.test_secondary_storage.TestSecStorageServices
integration.smoke.test_vm_life_cycle.TestDeployVM
integration.smoke.test_disk_offerings.TestCreateDiskOffering


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira closed the pull request at:

https://github.com/apache/cloudstack/pull/1285


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/1285#issuecomment-168905195
  
@remibergsma In the FS there is a requirement "Cloud admin should be able 
to to map AD OU / group to a Domain in CloudStack". So this means that LDAP 
group is mapped to a domain in CS. But this PR tries to map a group to CS 
account.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1285#issuecomment-167585886
  
LGTM, works as expected. 

![screen shot 2015-12-28 at 16 08 
34](https://cloud.githubusercontent.com/assets/1630096/12020487/55205136-ad7d-11e5-9c3d-ffe334270c4e.png)

@koushik-das I see your point and we can discuss. Although we don't break 
any of the functional requirements listed on the link you shared.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[borisroman]

Github user borisroman commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1285#discussion_r48488856
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
 ---
@@ -111,19 +114,27 @@ public LdapAuthenticator(final LdapManager 
ldapManager, final UserAccountDao 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();
+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 + ").");
--- End diff --

@miguelaferreira The following seems a bit strange "for LDAP group already 
exists not exist"... Maybe something like this; "for LDAP group already exists, 
but the user doesn't exist"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1285#discussion_r48496732
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
 ---
@@ -111,19 +114,27 @@ public LdapAuthenticator(final LdapManager 
ldapManager, final UserAccountDao 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();
+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 + ").");
--- End diff --

That's a copy & paste mistake. I'll fix it. Thanks for bringing it up


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[borisroman]

Github user borisroman commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1285#discussion_r48498393
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
 ---
@@ -111,19 +114,27 @@ public LdapAuthenticator(final LdapManager 
ldapManager, final UserAccountDao 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();
+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 + ").");
--- End diff --

NP


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map ldap group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira closed the pull request at:

https://github.com/apache/cloudstack/pull/1284


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira commented on the pull request:

https://github.com/apache/cloudstack/pull/1285#issuecomment-166936459
  
I've tested this PR manually in our beta environment, however ince there is 
a marvin test for this functionality I would also like to run that. I haven't 
yet because I haven't found a way to feed the required test data into the test 
run.

Advice on how to run the respective marvin test is very welcome.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map ldap group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira commented on the pull request:

https://github.com/apache/cloudstack/pull/1284#issuecomment-166936174
  
I've tested this PR manually in our beta environment, however ince there is 
a marvin test for this functionality I would also like to run that. I haven't 
yet because I haven't found a way to feed the required test data into the test 
run.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[miguelaferreira]

GitHub user miguelaferreira opened a pull request:

https://github.com/apache/cloudstack/pull/1285

Map LDAP group to Cloudstack account

The LDAP plugin authenticates Cloudstack users against users in an LDAP 
group, and when doing so creates a Cloudstack account with the same name as the 
LDAP user, and a Cloudstack user also with the same name as the LDAP user.
This makes it difficult to manage the users in a given LDAP group because 
each user will have it's own account and user in Cloudstack, and cannot 
therefore collaborate in managing cloud resources.

This PR changes the LDAP plugin to use the LDAP group for the Cloudstack 
account name, and create the users (belonging to the LDAP group) under the same 
Cloudstack account.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/miguelaferreira/cloudstack 
map-ldap-group-to-account

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1285.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1285


commit 42d0418530599717624f4614220d06c83ea48c2a
Author: Miguel Ferreira 
Date:   2015-12-23T13:49:29Z

Code formatting

commit 1ca4e484e0730f7a114dd4af7abcb2bcc11e7337
Author: Miguel Ferreira 
Date:   2015-12-23T13:49:53Z

Make ACS account using LDAP group name

commit 0089647c11a36e23cb89770f63c971ee8c12fe89
Author: Miguel Ferreira 
Date:   2015-12-23T14:16:48Z

Enable Java based unit tests in mvn lifecycle




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map ldap group to Cloudstack account

[miguelaferreira]

GitHub user miguelaferreira opened a pull request:

https://github.com/apache/cloudstack/pull/1284

Map ldap group to Cloudstack account

The LDAP plugin authenticates Cloudstack users against users in an LDAP 
group, and when doing so creates a Cloudstack account with the same name as the 
LDAP user, and a Cloudstack user also with the same name as the LDAP user.
This makes it difficult to manage the users in a given LDAP group because 
each user will have it's own account and user in Cloudstack, and cannot 
therefore collaborate in managing cloud resources.

This PR changes the LDAP plugin to use the LDAP group for the Cloudstack 
account name, and create the users (belonging to the LDAP group) under the same 
Cloudstack account.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/miguelaferreira/cloudstack 
map-ldap-group-to-account

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1284.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1284


commit 42d0418530599717624f4614220d06c83ea48c2a
Author: Miguel Ferreira 
Date:   2015-12-23T13:49:29Z

Code formatting

commit 1ca4e484e0730f7a114dd4af7abcb2bcc11e7337
Author: Miguel Ferreira 
Date:   2015-12-23T13:49:53Z

Make ACS account using LDAP group name

commit 0089647c11a36e23cb89770f63c971ee8c12fe89
Author: Miguel Ferreira 
Date:   2015-12-23T14:16:48Z

Enable Java based unit tests in mvn lifecycle




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map ldap group to Cloudstack account

[miguelaferreira]

Github user miguelaferreira commented on the pull request:

https://github.com/apache/cloudstack/pull/1284#issuecomment-166936278
  
Closing as this should have been directed at 4.7 branch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/1285#issuecomment-167044637
  
@miguelaferreira Looks like this PR is going to impact the feature 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/LDAP%3A+Trust+AD+and+Auto+Import
 that @karuturi added in 4.6. Please start a discussion thread at dev@ before 
proceeding further with this one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---