You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by miguelaferreira <gi...@git.apache.org> on 2015/12/23 17:26:18 UTC
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
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 <mi...@me.com>
Date: 2015-12-23T13:49:29Z
Code formatting
commit 1ca4e484e0730f7a114dd4af7abcb2bcc11e7337
Author: Miguel Ferreira <mi...@me.com>
Date: 2015-12-23T13:49:53Z
Make ACS account using LDAP group name
commit 0089647c11a36e23cb89770f63c971ee8c12fe89
Author: Miguel Ferreira <mi...@me.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by miguelaferreira <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by koushik-das <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by miguelaferreira <gi...@git.apache.org>.
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<Boolean, ActionOnFailedAuthentication>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by borisroman <gi...@git.apache.org>.
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<Boolean, ActionOnFailedAuthentication>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by koushik-das <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by borisroman <gi...@git.apache.org>.
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<Boolean, ActionOnFailedAuthentication>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by miguelaferreira <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1285#issuecomment-167585886
LGTM, works as expected.
@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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Map LDAP group to Cloudstack account
Posted by bvbharatk <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---