Browse Source

[useradmin] LDAP DNs should accept spaces

1. When a DN is being provided, it may have spaces. The corresponding username retrieved from LDAP should not have spaces nor colons.
2. When a username is provided, it should not have spaces nor colons.
3. Group names can have any character apparently.
Abraham Elmahrek 11 years ago
parent
commit
0ce3fbd

+ 37 - 22
apps/useradmin/src/useradmin/forms.py

@@ -41,6 +41,23 @@ def get_server_choices():
   else:
     return []
 
+def validate_dn(dn):
+  assert dn is not None, _('Full Distinguished Name required.')
+
+def validate_username(username_pattern):
+  validator = re.compile(r"^%s$" % get_username_re_rule())
+
+  assert username_pattern is not None, _('Username is required.')
+  assert len(username_pattern) <= 30, _('Username must be fewer than 30 characters.')
+  assert validator.match(username_pattern), _("Username must not contain whitespaces and ':'")
+
+def validate_groupname(groupname_pattern):
+  validator = re.compile(r"^%s$" % get_groupname_re_rule())
+
+  assert groupname_pattern is not None, _('Group name required.')
+  assert len(groupname_pattern) <= 80, _('Group name must be 80 characters or fewer.')
+  assert validator.match(groupname_pattern), _("Group name can be any character as long as it's 80 characters or fewer.")
+
 
 class UserChangeForm(django.contrib.auth.forms.UserChangeForm):
   """
@@ -117,9 +134,8 @@ class SuperUserChangeForm(UserChangeForm):
 
 
 class AddLdapUsersForm(forms.Form):
-  username_pattern = forms.RegexField(
+  username_pattern = forms.CharField(
       label=_t("Username"),
-      regex='^%s$' % (get_username_re_rule(),),
       help_text=_t("Required. 30 characters or fewer with username. 64 characters or fewer with DN. No whitespaces or colons."),
       error_messages={'invalid': _t("Whitespaces and ':' not allowed")})
   dn = forms.BooleanField(label=_t("Distinguished name"),
@@ -142,27 +158,23 @@ class AddLdapUsersForm(forms.Form):
     username_pattern = cleaned_data.get("username_pattern")
     dn = cleaned_data.get("dn")
 
-    if dn:
-      if username_pattern is not None and len(username_pattern) > 64:
-        msg = _('Too long: 64 characters or fewer and not %s.') % username_pattern
-        errors = self._errors.setdefault('username_pattern', ErrorList())
-        errors.append(msg)
-        raise forms.ValidationError(msg)
-    else:
-      if username_pattern is not None and len(username_pattern) > 30:
-        msg = _('Too long: 30 characters or fewer and not %s.') % username_pattern
-        errors = self._errors.setdefault('username_pattern', ErrorList())
-        errors.append(msg)
-        raise forms.ValidationError(msg)
+    try:
+      if dn:
+        validate_dn(username_pattern)
+      else:
+        validate_username(username_pattern)
+    except AssertionError, e:
+      errors = self._errors.setdefault('username_pattern', ErrorList())
+      errors.append(e.message)
+      raise forms.ValidationError(e.message)
 
     return cleaned_data
 
 
 class AddLdapGroupsForm(forms.Form):
-  groupname_pattern = forms.RegexField(
+  groupname_pattern = forms.CharField(
       label=_t("Name"),
       max_length=80,
-      regex='^%s$' % get_groupname_re_rule(),
       help_text=_t("Required. 80 characters or fewer."),
       error_messages={'invalid': _t("80 characters or fewer.") })
   dn = forms.BooleanField(label=_t("Distinguished name"),
@@ -193,12 +205,15 @@ class AddLdapGroupsForm(forms.Form):
     groupname_pattern = cleaned_data.get("groupname_pattern")
     dn = cleaned_data.get("dn")
 
-    if not dn:
-      if groupname_pattern is not None and len(groupname_pattern) > 80:
-        msg = _('Too long: 80 characters or fewer and not %s') % groupname_pattern
-        errors = self._errors.setdefault('groupname_pattern', ErrorList())
-        errors.append(msg)
-        raise forms.ValidationError(msg)
+    try:
+      if dn:
+        validate_dn(groupname_pattern)
+      else:
+        validate_groupname(groupname_pattern)
+    except AssertionError, e:
+      errors = self._errors.setdefault('groupname_pattern', ErrorList())
+      errors.append(e.message)
+      raise forms.ValidationError(e.message)
 
     return cleaned_data
 

+ 14 - 1
apps/useradmin/src/useradmin/test_ldap.py

@@ -511,7 +511,7 @@ def test_add_ldap_users():
     assert_true('Could not' in response.context['form'].errors['username_pattern'][0], response)
 
     # Test wild card
-    response = c.post(URL, dict(server='nonsense', username_pattern='*r*', password1='test', password2='test'))
+    response = c.post(URL, dict(server='nonsense', username_pattern='*rr*', password1='test', password2='test'))
     assert_true('/useradmin/users' in response['Location'], response)
 
     # Test ignore case
@@ -536,6 +536,19 @@ def test_add_ldap_users():
     assert_false(User.objects.filter(username='Rock').exists())
     assert_true(User.objects.filter(username='rock').exists())
 
+    # Test regular with spaces (should fail)
+    response = c.post(URL, dict(server='nonsense', username_pattern='user with space', password1='test', password2='test'))
+    assert_true("Username must not contain whitespaces and ':'" in response.context['form'].errors['username_pattern'][0], response)
+
+    # Test dn with spaces in username and dn (should fail)
+    response = c.post(URL, dict(server='nonsense', username_pattern='uid=user with space,ou=People,dc=example,dc=com', password1='test', password2='test', dn=True))
+    assert_true("There was a problem with some of the LDAP information" in response.content, response)
+    assert_true("Username must not contain whitespaces" in response.content, response)
+
+    # Test dn with spaces in dn, but not username (should succeed)
+    response = c.post(URL, dict(server='nonsense', username_pattern='uid=user without space,ou=People,dc=example,dc=com', password1='test', password2='test', dn=True))
+    assert_true(User.objects.filter(username='spaceless').exists())
+
   finally:
     for finish in done:
       finish()

+ 15 - 3
apps/useradmin/src/useradmin/test_ldap_deprecated.py

@@ -28,8 +28,7 @@ from django.conf import settings
 from django.contrib.auth.models import User, Group
 from django.core.urlresolvers import reverse
 
-from useradmin.models import LdapGroup, UserProfile
-from useradmin.models import get_profile
+from useradmin.models import LdapGroup, UserProfile, get_profile
 
 from hadoop import pseudo_hdfs4
 from views import sync_ldap_users, sync_ldap_groups, import_ldap_users, import_ldap_groups, \
@@ -469,7 +468,7 @@ def test_add_ldap_users():
     assert_true('Could not' in response.context['form'].errors['username_pattern'][0], response)
 
     # Test wild card
-    response = c.post(URL, dict(username_pattern='*r*', password1='test', password2='test'))
+    response = c.post(URL, dict(username_pattern='*rr*', password1='test', password2='test'))
     assert_true('/useradmin/users' in response['Location'], response)
 
     # Test ignore case
@@ -494,6 +493,19 @@ def test_add_ldap_users():
     assert_false(User.objects.filter(username='Rock').exists())
     assert_true(User.objects.filter(username='rock').exists())
 
+    # Test regular with spaces (should fail)
+    response = c.post(URL, dict(username_pattern='user with space', password1='test', password2='test'))
+    assert_true("Username must not contain whitespaces and ':'" in response.context['form'].errors['username_pattern'][0], response)
+
+    # Test dn with spaces in username and dn (should fail)
+    response = c.post(URL, dict(username_pattern='uid=user with space,ou=People,dc=example,dc=com', password1='test', password2='test', dn=True))
+    assert_true("There was a problem with some of the LDAP information" in response.content, response)
+    assert_true("Username must not contain whitespaces" in response.content, response)
+
+    # Test dn with spaces in dn, but not username (should succeed)
+    response = c.post(URL, dict(username_pattern='uid=user without space,ou=People,dc=example,dc=com', password1='test', password2='test', dn=True))
+    assert_true(User.objects.filter(username='spaceless').exists())
+
   finally:
     for finish in done:
       finish()

+ 3 - 1
apps/useradmin/src/useradmin/tests.py

@@ -152,7 +152,9 @@ class LdapTestConnection(object):
                     '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'}}
+                    'posix_person2': {'dn': 'uid=posix_person2,ou=People,dc=example,dc=com', 'username': 'posix_person2', 'first': 'pos', 'last': 'ix', 'email': 'pos@ix.com'},
+                    'user with space': {'dn': 'uid=user with space,ou=People,dc=example,dc=com', 'username': 'user with space', 'first': 'user', 'last': 'space', 'email': 'user@space.com'},
+                    'spaceless': {'dn': 'uid=user without space,ou=People,dc=example,dc=com', 'username': 'spaceless', 'first': 'user', 'last': 'space', 'email': 'user@space.com'},}
 
       self.groups = {'TestUsers': {
                         'dn': 'cn=TestUsers,ou=Groups,dc=example,dc=com',

+ 8 - 1
apps/useradmin/src/useradmin/views.py

@@ -41,7 +41,7 @@ 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, AddLdapUsersForm,\
-  PermissionsEditForm, GroupEditForm, SuperUserChangeForm, UserChangeForm
+  PermissionsEditForm, GroupEditForm, SuperUserChangeForm, UserChangeForm, validate_username
 
 
 
@@ -320,6 +320,8 @@ def add_ldap_users(request):
       except ldap.LDAPError, e:
         LOG.error("LDAP Exception: %s" % e)
         raise PopupException(_('There was an error when communicating with LDAP'), detail=str(e))
+      except AssertionError, e:
+        raise PopupException(_('There was a problem with some of the LDAP information'), detail=str(e))
 
       if users and form.cleaned_data['ensure_home_directory']:
         for user in users:
@@ -366,6 +368,8 @@ def add_ldap_groups(request):
       except ldap.LDAPError, e:
         LOG.error(_("LDAP Exception: %s") % e)
         raise PopupException(_('There was an error when communicating with LDAP'), detail=str(e))
+      except AssertionError, e:
+        raise PopupException(_('There was a problem with some of the LDAP information'), detail=str(e))
 
       if groups:
         return redirect(reverse(list_groups))
@@ -523,6 +527,9 @@ def _import_ldap_users_info(connection, user_info, sync_groups=False, import_by_
   """
   imported_users = []
   for ldap_info in user_info:
+    # Extra validation in case import by DN and username has spaces or colons
+    validate_username(ldap_info['username'])
+
     user, created = ldap_access.get_or_create_ldap_user(username=ldap_info['username'])
     profile = get_profile(user)
     if not created and profile.creation_method == str(UserProfile.CreationMethod.HUE):