Selaa lähdekoodia

[core] Nested members support

Support old subgroup methods by providing a switch.
Support nested members by recursively searching ldap tree.
Added tests validating nested group import.
Abraham Elmahrek 11 vuotta sitten
vanhempi
commit
0f151d0

+ 7 - 4
apps/useradmin/src/useradmin/forms.py

@@ -161,10 +161,6 @@ class AddLdapGroupsForm(forms.Form):
                                       help_text=_t('Import unimported or new users from the group.'),
                                       initial=False,
                                       required=False)
-  import_members_recursive = forms.BooleanField(label=_t('Import new members from all subgroups'),
-                                                help_text=_t('Import unimported or new users from the all subgroups.'),
-                                                initial=False,
-                                                required=False)
   ensure_home_directories = forms.BooleanField(label=_t('Create home directories'),
                                                 help_text=_t('Create home directories for every member imported, if members are being imported.'),
                                                 initial=True,
@@ -185,6 +181,13 @@ class AddLdapGroupsForm(forms.Form):
     return cleaned_data
 
 
+class AddLdapSuboordinateGroupsForm(AddLdapGroupsForm):
+  import_members_recursive = forms.BooleanField(label=_t('Import new members from all subgroups'),
+                                                help_text=_t('Import unimported or new users from the all subgroups.'),
+                                                initial=False,
+                                                required=False)
+
+
 class GroupEditForm(forms.ModelForm):
   """
   Form to manipulate a group.  This manages the group name and its membership.

+ 33 - 0
apps/useradmin/src/useradmin/ldap_access.py

@@ -308,6 +308,39 @@ class LdapConnection(object):
     else:
       return []
 
+  def find_members_of_group(self, dn, search_attr, ldap_filter, scope=ldap.SCOPE_SUBTREE):
+    if ldap_filter and not ldap_filter.startswith('('):
+      ldap_filter = '(' + ldap_filter + ')'
+
+    # Allow wild cards on non distinguished names
+    dn = ldap.filter.escape_filter_chars(dn).replace(r'\2a', r'*')
+    # Fix issue where \, is converted to \5c,
+    dn = dn.replace(r'\5c,', r'\2c')
+
+    search_dn, _ = self._get_search_params(dn, search_attr)
+    ldap_filter = '(&%(ldap_filter)s(|(isMemberOf=%(group_dn)s)(memberOf=%(group_dn)s)))' % {'group_dn': dn, 'ldap_filter': ldap_filter}
+    attrlist = ['objectClass', 'isMemberOf', 'memberOf', 'givenName', 'sn', 'mail', 'dn', search_attr]
+
+    ldap_result_id = self.ldap_handle.search(search_dn, scope, ldap_filter, attrlist)
+    result_type, result_data = self.ldap_handle.result(ldap_result_id)
+
+    if result_type == ldap.RES_SEARCH_RESULT:
+      return result_data
+    else:
+      return []
+
+  def find_users_of_group(self, dn):
+    ldap_filter = desktop.conf.LDAP.USERS.USER_FILTER.get()
+    name_attr = desktop.conf.LDAP.USERS.USER_NAME_ATTR.get()
+    result_data = self.find_members_of_group(dn, name_attr, ldap_filter)
+    return self._transform_find_user_results(result_data, name_attr)
+
+  def find_groups_of_group(self, dn):
+    ldap_filter = desktop.conf.LDAP.GROUPS.GROUP_FILTER.get()
+    name_attr = desktop.conf.LDAP.GROUPS.GROUP_NAME_ATTR.get()
+    result_data = self.find_members_of_group(dn, name_attr, ldap_filter)
+    return self._transform_find_group_results(result_data, name_attr)
+
   def _get_root_dn(self):
     """
     Returns the configured base DN (DC=desktop,DC=local).

+ 249 - 98
apps/useradmin/src/useradmin/tests.py

@@ -104,12 +104,59 @@ class LdapTestConnection(object):
       data = [self._instance.groups.get(groupname) for groupname in groupnames]
     return data
 
+  def find_members_of_group(self, dn, search_attr, ldap_filter, scope=ldap.SCOPE_SUBTREE):
+    members = []
+    for group_info in self._instance.groups:
+      if group_info['dn'] == dn:
+        members.extend(group_info['members'])
+
+    members = set(members)
+    users = []
+    for user_info in self._instance.users:
+      if user_info['dn'] in members:
+        users.append(user_info)
+
+    groups = []
+    for group_info in self._instance.groups:
+      if group_info['dn'] in members:
+        groups.append(group_info)
+
+    return users + groups
+
+  def find_users_of_group(self, dn):
+    members = []
+    for group_info in self._instance.groups.values():
+      if group_info['dn'] == dn:
+        members.extend(group_info['members'])
+
+    members = set(members)
+    users = []
+    for user_info in self._instance.users.values():
+      if user_info['dn'] in members:
+        users.append(user_info)
+
+    return users
+
+  def find_groups_of_group(self, dn):
+    members = []
+    for group_info in self._instance.groups.values():
+      if group_info['dn'] == dn:
+        members.extend(group_info['members'])
+
+    groups = []
+    for group_info in self._instance.groups.values():
+      if group_info['dn'] in members:
+        groups.append(group_info)
+
+    return groups
+
   class Data:
     def __init__(self):
       self.users = {'moe': {'dn': 'uid=moe,ou=People,dc=example,dc=com', 'username':'moe', 'first':'Moe', 'email':'moe@stooges.com', 'groups': ['cn=TestUsers,ou=Groups,dc=example,dc=com']},
                     'lårry': {'dn': 'uid=lårry,ou=People,dc=example,dc=com', 'username':'lårry', 'first':'Larry', 'last':'Stooge', 'email':'larry@stooges.com', 'groups': ['cn=TestUsers,ou=Groups,dc=example,dc=com', 'cn=Test Administrators,cn=TestUsers,ou=Groups,dc=example,dc=com']},
                     'curly': {'dn': 'uid=curly,ou=People,dc=example,dc=com', 'username':'curly', 'first':'Curly', 'last':'Stooge', 'email':'curly@stooges.com', 'groups': ['cn=TestUsers,ou=Groups,dc=example,dc=com', 'cn=Test Administrators,cn=TestUsers,ou=Groups,dc=example,dc=com']},
                     'Rock': {'dn': 'uid=Rock,ou=People,dc=example,dc=com', 'username':'Rock', 'first':'rock', 'last':'man', 'email':'rockman@stooges.com', 'groups': ['cn=Test Administrators,cn=TestUsers,ou=Groups,dc=example,dc=com']},
+                    'nestedguy': {'dn': 'uid=nestedguy,ou=People,dc=example,dc=com', 'username':'nestedguy', 'first':'nested', 'last':'guy', 'email':'nestedguy@stooges.com', 'groups': ['cn=NestedGroup,ou=Groups,dc=example,dc=com']},
                     'otherguy': {'dn': 'uid=otherguy,ou=People,dc=example,dc=com', 'username':'otherguy', 'first':'Other', 'last':'Guy', 'email':'other@guy.com'},
                     'posix_person': {'dn': 'uid=posix_person,ou=People,dc=example,dc=com', 'username': 'posix_person', 'first': 'pos', 'last': 'ix', 'email': 'pos@ix.com'},
                     'posix_person2': {'dn': 'uid=posix_person2,ou=People,dc=example,dc=com', 'username': 'posix_person2', 'first': 'pos', 'last': 'ix', 'email': 'pos@ix.com'}}
@@ -119,22 +166,34 @@ class LdapTestConnection(object):
                         'name':'TestUsers',
                         'members':['uid=moe,ou=People,dc=example,dc=com','uid=lårry,ou=People,dc=example,dc=com','uid=curly,ou=People,dc=example,dc=com'],
                         'posix_members':[]},
-                     'Test Administrators': {
+                      'Test Administrators': {
                         'dn': 'cn=Test Administrators,cn=TestUsers,ou=Groups,dc=example,dc=com',
                         'name':'Test Administrators',
                         'members':['uid=Rock,ou=People,dc=example,dc=com','uid=lårry,ou=People,dc=example,dc=com','uid=curly,ou=People,dc=example,dc=com'],
                         'posix_members':[]},
-                     'OtherGroup': {
+                      'OtherGroup': {
                         'dn': 'cn=OtherGroup,cn=TestUsers,ou=Groups,dc=example,dc=com',
                         'name':'OtherGroup',
                         'members':[],
                         'posix_members':[]},
-                     'PosixGroup': {
+                      'NestedGroups': {
+                        'dn': 'cn=NestedGroups,ou=Groups,dc=example,dc=com',
+                        'name':'NestedGroups',
+                        'members':['cn=NestedGroup,ou=Groups,dc=example,dc=com'],
+                        'posix_members':[]
+                      },
+                      'NestedGroup': {
+                        'dn': 'cn=NestedGroup,ou=Groups,dc=example,dc=com',
+                        'name':'NestedGroup',
+                        'members':['uid=nestedguy,ou=People,dc=example,dc=com'],
+                        'posix_members':[]
+                      },
+                      'PosixGroup': {
                         'dn': 'cn=PosixGroup,ou=Groups,dc=example,dc=com',
                         'name':'PosixGroup',
                         'members':[],
                         'posix_members':['posix_person','lårry']},
-                     'PosixGroup1': {
+                      'PosixGroup1': {
                         'dn': 'cn=PosixGroup1,cn=PosixGroup,ou=Groups,dc=example,dc=com',
                         'name':'PosixGroup1',
                         'members':[],
@@ -509,112 +568,204 @@ def test_useradmin_ldap_group_integration():
   reset_all_users()
   reset_all_groups()
 
+  reset = []
+
+  # Set up LDAP tests to use a LdapTestConnection instead of an actual LDAP connection
+  ldap_access.CACHED_LDAP_CONN = LdapTestConnection()
+
+  # Test old subgroups
+  reset.append(desktop.conf.LDAP.SUBGROUPS.set_for_testing("suboordinate"))
+
+  try:
+    # Import groups only
+    import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=False, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 0)
+
+    # Import all members of TestUsers
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 3)
+
+    # Should import a group, but will only sync already-imported members
+    import_ldap_groups('Test Administrators', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(User.objects.all().count(), 3)
+    assert_equal(Group.objects.all().count(), 2)
+    test_admins = Group.objects.get(name='Test Administrators')
+    assert_equal(test_admins.user_set.all().count(), 2)
+    larry = User.objects.get(username='lårry')
+    assert_equal(test_admins.user_set.all()[0].username, larry.username)
+
+    # Only sync already imported
+    ldap_access.CACHED_LDAP_CONN.remove_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
+    import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 2)
+    assert_equal(User.objects.get(username='moe').groups.all().count(), 0)
+
+    # Import missing user
+    ldap_access.CACHED_LDAP_CONN.add_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 3)
+    assert_equal(User.objects.get(username='moe').groups.all().count(), 1)
+
+    # Import all members of TestUsers and members of subgroups
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=True, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 4)
+
+    # Make sure Hue groups with naming collisions don't get marked as LDAP groups
+    hue_user = User.objects.create(username='otherguy', first_name='Different', last_name='Guy')
+    hue_group = Group.objects.create(name='OtherGroup')
+    hue_group.user_set.add(hue_user)
+    hue_group.save()
+    import_ldap_groups('OtherGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_false(LdapGroup.objects.filter(group=hue_group).exists())
+    assert_true(hue_group.user_set.filter(username=hue_user.username).exists())
+  finally:
+    for finish in reset:
+      finish()
+
+
+def test_useradmin_ldap_nested_group_integration():
+  reset_all_users()
+  reset_all_groups()
+
+  reset = []
+
   # Set up LDAP tests to use a LdapTestConnection instead of an actual LDAP connection
   ldap_access.CACHED_LDAP_CONN = LdapTestConnection()
 
-  # Import groups only
-  import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=False, import_by_dn=False)
-  test_users = Group.objects.get(name='TestUsers')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 0)
-
-  # Import all members of TestUsers
-  import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  test_users = Group.objects.get(name='TestUsers')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 3)
-
-  # Should import a group, but will only sync already-imported members
-  import_ldap_groups('Test Administrators', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(User.objects.all().count(), 3)
-  assert_equal(Group.objects.all().count(), 2)
-  test_admins = Group.objects.get(name='Test Administrators')
-  assert_equal(test_admins.user_set.all().count(), 2)
-  larry = User.objects.get(username='lårry')
-  assert_equal(test_admins.user_set.all()[0].username, larry.username)
-
-  # Only sync already imported
-  ldap_access.CACHED_LDAP_CONN.remove_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
-  import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(test_users.user_set.all().count(), 2)
-  assert_equal(User.objects.get(username='moe').groups.all().count(), 0)
-
-  # Import missing user
-  ldap_access.CACHED_LDAP_CONN.add_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
-  import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(test_users.user_set.all().count(), 3)
-  assert_equal(User.objects.get(username='moe').groups.all().count(), 1)
-
-  # Import all members of TestUsers and members of subgroups
-  import_ldap_groups('TestUsers', import_members=True, import_members_recursive=True, sync_users=True, import_by_dn=False)
-  test_users = Group.objects.get(name='TestUsers')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 4)
-
-  # Make sure Hue groups with naming collisions don't get marked as LDAP groups
-  hue_user = User.objects.create(username='otherguy', first_name='Different', last_name='Guy')
-  hue_group = Group.objects.create(name='OtherGroup')
-  hue_group.user_set.add(hue_user)
-  hue_group.save()
-  import_ldap_groups('OtherGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_false(LdapGroup.objects.filter(group=hue_group).exists())
-  assert_true(hue_group.user_set.filter(username=hue_user.username).exists())
+  # Test old subgroups
+  reset.append(desktop.conf.LDAP.SUBGROUPS.set_for_testing("nested"))
+
+  try:
+    # Import groups only
+    import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=False, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 0)
+
+    # Import all members of TestUsers
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 3)
+
+    # Should import a group, but will only sync already-imported members
+    import_ldap_groups('Test Administrators', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(User.objects.all().count(), 3)
+    assert_equal(Group.objects.all().count(), 2)
+    test_admins = Group.objects.get(name='Test Administrators')
+    assert_equal(test_admins.user_set.all().count(), 2)
+    larry = User.objects.get(username='lårry')
+    assert_equal(test_admins.user_set.all()[0].username, larry.username)
+
+    # Only sync already imported
+    ldap_access.CACHED_LDAP_CONN.remove_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
+    import_ldap_groups('TestUsers', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 2)
+    assert_equal(User.objects.get(username='moe').groups.all().count(), 0)
+
+    # Import missing user
+    ldap_access.CACHED_LDAP_CONN.add_user_group_for_test('uid=moe,ou=People,dc=example,dc=com', 'TestUsers')
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 3)
+    assert_equal(User.objects.get(username='moe').groups.all().count(), 1)
+
+    # Import all members of TestUsers and not members of suboordinate groups (even though specified)
+    import_ldap_groups('TestUsers', import_members=True, import_members_recursive=True, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='TestUsers')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 3)
+
+    # Nested group import
+    import_ldap_groups('NestedGroups', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    nested_groups = Group.objects.get(name='NestedGroups')
+    nested_group = Group.objects.get(name='NestedGroup')
+    assert_true(LdapGroup.objects.filter(group=nested_groups).exists())
+    assert_true(LdapGroup.objects.filter(group=nested_group).exists())
+    assert_equal(nested_groups.user_set.all().count(), 0)
+    assert_equal(nested_group.user_set.all().count(), 1)
+
+    # Make sure Hue groups with naming collisions don't get marked as LDAP groups
+    hue_user = User.objects.create(username='otherguy', first_name='Different', last_name='Guy')
+    hue_group = Group.objects.create(name='OtherGroup')
+    hue_group.user_set.add(hue_user)
+    hue_group.save()
+    import_ldap_groups('OtherGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_false(LdapGroup.objects.filter(group=hue_group).exists())
+    assert_true(hue_group.user_set.filter(username=hue_user.username).exists())
+  finally:
+    for finish in reset:
+      finish()
 
 
 def test_useradmin_ldap_posix_group_integration():
   reset_all_users()
   reset_all_groups()
 
+  reset = []
+
   # Set up LDAP tests to use a LdapTestConnection instead of an actual LDAP connection
   ldap_access.CACHED_LDAP_CONN = LdapTestConnection()
 
-  # Import groups only
-  import_ldap_groups('PosixGroup', import_members=False, import_members_recursive=False, sync_users=False, import_by_dn=False)
-  test_users = Group.objects.get(name='PosixGroup')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 0)
-
-  # Import all members of TestUsers
-  import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  test_users = Group.objects.get(name='PosixGroup')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 2)
-
-  # Should import a group, but will only sync already-imported members
-  import_ldap_groups('Test Administrators', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(User.objects.all().count(), 2, User.objects.all())
-  assert_equal(Group.objects.all().count(), 2, Group.objects.all())
-  test_admins = Group.objects.get(name='Test Administrators')
-  assert_equal(test_admins.user_set.all().count(), 1)
-  larry = User.objects.get(username='lårry')
-  assert_equal(test_admins.user_set.all()[0].username, larry.username)
-
-  # Only sync already imported
-  ldap_access.CACHED_LDAP_CONN.remove_posix_user_group_for_test('posix_person', 'PosixGroup')
-  import_ldap_groups('PosixGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(test_users.user_set.all().count(), 1)
-  assert_equal(User.objects.get(username='posix_person').groups.all().count(), 0)
-
-  # Import missing user
-  ldap_access.CACHED_LDAP_CONN.add_posix_user_group_for_test('posix_person', 'PosixGroup')
-  import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_equal(test_users.user_set.all().count(), 2)
-  assert_equal(User.objects.get(username='posix_person').groups.all().count(), 1)
-
-  # Import all members of PosixGroup and members of subgroups
-  import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=True, sync_users=True, import_by_dn=False)
-  test_users = Group.objects.get(name='PosixGroup')
-  assert_true(LdapGroup.objects.filter(group=test_users).exists())
-  assert_equal(test_users.user_set.all().count(), 3)
-
-  # Make sure Hue groups with naming collisions don't get marked as LDAP groups
-  hue_user = User.objects.create(username='otherguy', first_name='Different', last_name='Guy')
-  hue_group = Group.objects.create(name='OtherGroup')
-  hue_group.user_set.add(hue_user)
-  hue_group.save()
-  import_ldap_groups('OtherGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
-  assert_false(LdapGroup.objects.filter(group=hue_group).exists())
-  assert_true(hue_group.user_set.filter(username=hue_user.username).exists())
+  # Test old subgroups
+  reset.append(desktop.conf.LDAP.SUBGROUPS.set_for_testing("suboordinate"))
+
+  try:
+    # Import groups only
+    import_ldap_groups('PosixGroup', import_members=False, import_members_recursive=False, sync_users=False, import_by_dn=False)
+    test_users = Group.objects.get(name='PosixGroup')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 0)
+
+    # Import all members of TestUsers
+    import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='PosixGroup')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 2)
+
+    # Should import a group, but will only sync already-imported members
+    import_ldap_groups('Test Administrators', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(User.objects.all().count(), 2, User.objects.all())
+    assert_equal(Group.objects.all().count(), 2, Group.objects.all())
+    test_admins = Group.objects.get(name='Test Administrators')
+    assert_equal(test_admins.user_set.all().count(), 1)
+    larry = User.objects.get(username='lårry')
+    assert_equal(test_admins.user_set.all()[0].username, larry.username)
+
+    # Only sync already imported
+    ldap_access.CACHED_LDAP_CONN.remove_posix_user_group_for_test('posix_person', 'PosixGroup')
+    import_ldap_groups('PosixGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 1)
+    assert_equal(User.objects.get(username='posix_person').groups.all().count(), 0)
+
+    # Import missing user
+    ldap_access.CACHED_LDAP_CONN.add_posix_user_group_for_test('posix_person', 'PosixGroup')
+    import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_equal(test_users.user_set.all().count(), 2)
+    assert_equal(User.objects.get(username='posix_person').groups.all().count(), 1)
+
+    # Import all members of PosixGroup and members of subgroups
+    import_ldap_groups('PosixGroup', import_members=True, import_members_recursive=True, sync_users=True, import_by_dn=False)
+    test_users = Group.objects.get(name='PosixGroup')
+    assert_true(LdapGroup.objects.filter(group=test_users).exists())
+    assert_equal(test_users.user_set.all().count(), 3)
+
+    # Make sure Hue groups with naming collisions don't get marked as LDAP groups
+    hue_user = User.objects.create(username='otherguy', first_name='Different', last_name='Guy')
+    hue_group = Group.objects.create(name='OtherGroup')
+    hue_group.user_set.add(hue_user)
+    hue_group.save()
+    import_ldap_groups('OtherGroup', import_members=False, import_members_recursive=False, sync_users=True, import_by_dn=False)
+    assert_false(LdapGroup.objects.filter(group=hue_group).exists())
+    assert_true(hue_group.user_set.filter(username=hue_user.username).exists())
+  finally:
+    for finish in reset:
+      finish()
 
 
 def test_useradmin_ldap_user_integration():

+ 85 - 47
apps/useradmin/src/useradmin/views.py

@@ -29,6 +29,7 @@ from django.contrib.auth.models import User, Group
 from desktop.lib.django_util import render
 from desktop.lib.exceptions_renderable import PopupException
 from django.core.urlresolvers import reverse
+from django.utils.encoding import smart_str
 from django.utils.translation import ugettext as _
 from django.forms.util import ErrorList
 from django.shortcuts import redirect
@@ -39,7 +40,7 @@ from desktop.conf import LDAP
 from hadoop.fs.exceptions import WebHdfsException
 from useradmin.models import HuePermission, UserProfile, LdapGroup
 from useradmin.models import get_profile, get_default_user_group
-from useradmin.forms import SyncLdapUsersGroupsForm, AddLdapGroupsForm,\
+from useradmin.forms import SyncLdapUsersGroupsForm, AddLdapGroupsForm,AddLdapSuboordinateGroupsForm,\
   AddLdapUsersForm, PermissionsEditForm, GroupEditForm, SuperUserChangeForm,\
   UserChangeForm
 
@@ -348,13 +349,18 @@ def add_ldap_groups(request):
   if not request.user.is_superuser:
     raise PopupException(_("You must be a superuser to add another group."), error_code=401)
 
+  if desktop.conf.LDAP.SUBGROUPS.get() == 'suboordinate':
+    FormClass = AddLdapSuboordinateGroupsForm
+  else:
+    FormClass = AddLdapGroupsForm
+
   if request.method == 'POST':
-    form = AddLdapGroupsForm(request.POST)
+    form = FormClass(request.POST)
     if form.is_valid():
       groupname_pattern = form.cleaned_data['groupname_pattern']
       import_by_dn = form.cleaned_data['dn']
       import_members = form.cleaned_data['import_members']
-      import_members_recursive = form.cleaned_data['import_members_recursive']
+      import_members_recursive = form.cleaned_data.get('import_members_recursive', False)
       try:
         groups = import_ldap_groups(groupname_pattern, import_members=import_members, import_members_recursive=import_members_recursive, sync_users=True, import_by_dn=import_by_dn)
       except ldap.LDAPError, e:
@@ -368,7 +374,7 @@ def add_ldap_groups(request):
         errors.append(_('Could not get LDAP details for groups in pattern %s') % groupname_pattern)
 
   else:
-    form = AddLdapGroupsForm()
+    form = FormClass()
 
   return render('edit_group.mako', request, dict(form=form, action=request.path, ldap=True))
 
@@ -568,6 +574,37 @@ def _import_ldap_users_info(user_info, sync_groups=False, import_by_dn=False):
   return imported_users
 
 
+def _import_ldap_members(conn, group, ldap_info, count=0):
+  if count >= desktop.conf.LDAP.NESTED_MEMBERS_SEARCH_DEPTH.get():
+    return None
+
+  user_set = set()
+  group_set = set()
+
+  # Find all users and groups of group.
+  users_info = conn.find_users_of_group(ldap_info['dn'])
+  groups_info = conn.find_groups_of_group(ldap_info['dn'])
+  for user_info in users_info:
+    LOG.debug("Importing user %s" % smart_str(user_info['dn']))
+    users = _import_ldap_users(smart_str(user_info['dn']), import_by_dn=True)
+    group.user_set.add(*users)
+    user_set |= set(users)
+  for group_info in groups_info:
+    LOG.debug("Importing group %s" % smart_str(group_info['dn']))
+    group_set |= set(_import_ldap_groups(smart_str(group_info['dn']), import_by_dn=True))
+
+    # Must find all members of subgroups
+    if len(group_set) > 1:
+      LOG.warn('Found multiple groups for member %s.' % smart_str(group_info['dn']))
+    else:
+      for group in group_set:
+        new_user_set, new_group_set = _import_ldap_members(conn, group, group_info, count+1)
+        user_set |= new_user_set
+        group_set |= new_group_set
+
+  return user_set, group_set
+
+
 def _import_ldap_groups(groupname_pattern, import_members=False, recursive_import_members=False, sync_users=True, import_by_dn=False):
   """
   Import a group from LDAP. If import_members is true, this will also import any
@@ -601,21 +638,26 @@ def _import_ldap_groups(groupname_pattern, import_members=False, recursive_impor
     # Find members and posix members for group and subgoups
     members = ldap_info['members']
     posix_members = ldap_info['posix_members']
-    if recursive_import_members:
-      sub_group_info = conn.find_groups(ldap_info['dn'], find_by_dn=True)
-      for sub_ldap_info in sub_group_info:
-        members.extend(sub_ldap_info['members'])
-        posix_members.extend(sub_ldap_info['posix_members'])
-
-    # Import/fetch users
-    for member in members:
-      users = []
 
+    # @TODO: Deprecate recursive_import_members as it may not be useful.
+    if desktop.conf.LDAP.SUBGROUPS.get() == 'suboordinate':
       if import_members:
-        LOG.debug("Importing user %s" % str(member))
-        users = _import_ldap_users(member, import_by_dn=True)
+        if recursive_import_members:
+          for sub_ldap_info in conn.find_groups(ldap_info['dn'], find_by_dn=True):
+            members += sub_ldap_info['members']
+            posix_members += sub_ldap_info['posix_members']
+
+        for member in members:
+          LOG.debug("Importing user %s" % smart_str(member))
+          group.user_set.add( *( _import_ldap_users(member, import_by_dn=True) or [] ) )
+    else:
+      # Import users and groups
+      if import_members:
+        new_users_set, new_groups_set = _import_ldap_members(conn, group, ldap_info)
 
-      elif sync_users:
+    # Sync users
+    if sync_users:
+      for member in members:
         user_info = conn.find_users(member, find_by_dn=True)
         if len(user_info) > 1:
           LOG.warn('Found multiple users for member %s.' % member)
@@ -623,41 +665,37 @@ def _import_ldap_groups(groupname_pattern, import_members=False, recursive_impor
           for ldap_info in user_info:
             try:
               user = ldap_access.get_ldap_user(username=ldap_info['username'])
-              users.append(user)
+              group.user_set.add(user)
             except User.DoesNotExist:
               pass
 
-      if users:
-        LOG.debug("Adding member %s represented as users (should be a single user) %s to group %s" % (str(member), str(users), str(group.name)))
-        group.user_set.add(*users)
-
-
-    # Import/fetch posix users
-    for posix_member in posix_members:
-      users = []
-
+    # Import/fetch posix users and groups
+    # Posix members
+    if posix_members:
       if import_members:
-        LOG.debug("Importing user %s" % str(posix_member))
-        # posixGroup class defines 'memberUid' to be login names,
-        # which are defined by 'uid'.
-        user_info = conn.find_users(posix_member, search_attr='uid', user_name_attr=desktop.conf.LDAP.USERS.USER_NAME_ATTR.get(), find_by_dn=False)
-        users = _import_ldap_users_info(user_info, import_by_dn=False)
-
-      elif sync_users:
-        user_info = conn.find_users(posix_member, search_attr='uid', user_name_attr=desktop.conf.LDAP.USERS.USER_NAME_ATTR.get(), find_by_dn=False)
-        if len(user_info) > 1:
-          LOG.warn('Found multiple users for member %s.' % member)
-        else:
-          for ldap_info in user_info:
-            try:
-              user = ldap_access.get_ldap_user(username=ldap_info['username'])
-              users.append(user)
-            except User.DoesNotExist:
-              pass
-
-      if users:
-        LOG.debug("Adding member %s represented as users (should be a single user) %s to group %s" % (str(posix_member), str(users), str(group.name)))
-        group.user_set.add(*users)
+        for posix_member in posix_members:
+          LOG.debug("Importing user %s" % str(posix_member))
+          # posixGroup class defines 'memberUid' to be login names,
+          # which are defined by 'uid'.
+          user_info = conn.find_users(posix_member, search_attr='uid', user_name_attr=desktop.conf.LDAP.USERS.USER_NAME_ATTR.get(), find_by_dn=False)
+          users = _import_ldap_users_info(user_info, import_by_dn=False)
+
+          if users:
+            LOG.debug("Adding member %s represented as users (should be a single user) %s to group %s" % (str(posix_member), str(users), str(group.name)))
+            group.user_set.add(*users)
+
+      if sync_users:
+        for posix_member in posix_members:
+          user_info = conn.find_users(posix_member, search_attr='uid', user_name_attr=desktop.conf.LDAP.USERS.USER_NAME_ATTR.get(), find_by_dn=False)
+          if len(user_info) > 1:
+            LOG.warn('Found multiple users for member %s.' % posix_member)
+          else:
+            for ldap_info in user_info:
+              try:
+                user = ldap_access.get_ldap_user(username=ldap_info['username'])
+                group.user_set.add(user)
+              except User.DoesNotExist:
+                pass
 
     group.save()
     groups.append(group)

+ 13 - 1
desktop/core/src/desktop/conf.py

@@ -23,7 +23,7 @@ from django.utils.translation import ugettext_lazy as _
 
 from desktop.lib.conf import Config, ConfigSection, UnspecifiedConfigSection,\
                              coerce_bool, coerce_csv, coerce_json_dict,\
-                             validate_path, list_of_compiled_res
+                             validate_path, list_of_compiled_res, coerce_str_lowercase
 from desktop.lib.i18n import force_unicode
 from desktop.lib.paths import get_desktop_root
 
@@ -457,8 +457,20 @@ LDAP = ConfigSection(
     FORCE_USERNAME_LOWERCASE = Config("force_username_lowercase",
       help=_("Force usernames to lowercase when creating new users from LDAP."),
       type=coerce_bool,
+      private=True,
       default=False),
 
+    SUBGROUPS = Config("subgroups",
+      private=True,
+      help=_("Choose which kind of subgrouping to use: nested or suboordinate (deprecated)."),
+      type=coerce_str_lowercase,
+      default="nested"),
+    NESTED_MEMBERS_SEARCH_DEPTH = Config("nested_members_search_depth",
+      private=True,
+      help=_("Define the number of levels to search for nested members."),
+      type=int,
+      default=10),
+
     USERS = ConfigSection(
       key="users",
       help=_("Configuration for LDAP user schema and search."),

+ 7 - 0
desktop/core/src/desktop/lib/conf.py

@@ -65,6 +65,8 @@ variables.
 # using it. So instead of breaking compatibility, we make a "pytype" alias.
 pytype = type
 
+from django.utils.encoding import smart_str
+
 from desktop.lib.paths import get_desktop_root, get_build_dir
 
 import configobj
@@ -599,6 +601,11 @@ def initialize(modules, config_dir):
 def is_anonymous(key):
   return key == _ANONYMOUS
 
+
+def coerce_str_lowercase(value):
+  return smart_str(value).lower()
+
+
 def coerce_bool(value):
   if isinstance(value, bool):
     return value