Przeglądaj źródła

HUE-3049 [useradmin] Change errors in validation functions from AssertionError to ValidationError

useradmin forms use asserts for validating fields, but should raise ValidationError since AssertionError doesn't get caught by python in optimized mode
Jenny Kim 10 lat temu
rodzic
commit
3618f09

+ 17 - 9
apps/useradmin/src/useradmin/forms.py

@@ -21,6 +21,7 @@ import re
 import django.contrib.auth.forms
 from django import forms
 from django.contrib.auth.models import User, Group
+from django.forms import ValidationError
 from django.forms.util import ErrorList
 from django.utils.translation import ugettext as _, ugettext_lazy as _t
 
@@ -43,21 +44,28 @@ def get_server_choices():
     return []
 
 def validate_dn(dn):
-  assert dn is not None, _('Full Distinguished Name required.')
+  if not dn:
+    raise ValidationError(_('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 ':'")
+  if not username_pattern:
+    raise ValidationError(_('Username is required.'))
+  if len(username_pattern) > 30:
+    raise ValidationError(_('Username must be fewer than 30 characters.'))
+  if not validator.match(username_pattern):
+    raise ValidationError(_("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.")
+  if not groupname_pattern:
+    raise ValidationError(_('Group name required.'))
+  if len(groupname_pattern) > 80:
+    raise ValidationError(_('Group name must be 80 characters or fewer.'))
+  if not validator.match(groupname_pattern):
+    raise ValidationError(_("Group name can be any character as long as it's 80 characters or fewer."))
 
 
 class UserChangeForm(django.contrib.auth.forms.UserChangeForm):
@@ -208,7 +216,7 @@ class AddLdapUsersForm(forms.Form):
         validate_dn(username_pattern)
       else:
         validate_username(username_pattern)
-    except AssertionError, e:
+    except ValidationError, e:
       errors = self._errors.setdefault('username_pattern', ErrorList())
       errors.append(e.message)
       raise forms.ValidationError(e.message)
@@ -255,7 +263,7 @@ class AddLdapGroupsForm(forms.Form):
         validate_dn(groupname_pattern)
       else:
         validate_groupname(groupname_pattern)
-    except AssertionError, e:
+    except ValidationError, e:
       errors = self._errors.setdefault('groupname_pattern', ErrorList())
       errors.append(e.message)
       raise forms.ValidationError(e.message)

+ 14 - 12
apps/useradmin/src/useradmin/views.py

@@ -27,18 +27,19 @@ import ldap_access
 from ldap_access import LdapSearchException
 
 from django.contrib.auth.models import User, Group
-
-from desktop.lib.django_util import JsonResponse, 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 import ValidationError
 from django.forms.util import ErrorList
-from django.shortcuts import redirect
 from django.http import HttpResponse
+from django.shortcuts import redirect
+from django.utils.encoding import smart_str
+from django.utils.translation import ugettext as _
 
 import desktop.conf
 from desktop.conf import LDAP
+from desktop.lib.django_util import JsonResponse, render
+from desktop.lib.exceptions_renderable import PopupException
+
 from hadoop.fs.exceptions import WebHdfsException
 from useradmin.models import HuePermission, UserProfile, LdapGroup
 from useradmin.models import get_profile, get_default_user_group
@@ -356,7 +357,7 @@ 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:
+      except ValidationError, 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']:
@@ -408,7 +409,7 @@ 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:
+      except ValidationError, e:
         raise PopupException(_('There was a problem with some of the LDAP information'), detail=str(e))
 
       unique_users = set()
@@ -591,7 +592,8 @@ def _check_remove_last_super(user_obj):
   all_active_su = User.objects.filter(is_superuser__exact = True,
                                       is_active__exact = True)
   num_active_su = all_active_su.count()
-  assert num_active_su >= 1, _("No active superuser configured.")
+  if num_active_su < 1:
+    raise PopupException(_("No active superuser configured."))
   if num_active_su == 1:
     raise PopupException(_("You cannot remove the last active superuser from the configuration."), error_code=401)
 
@@ -687,7 +689,7 @@ def _import_ldap_users_info(connection, user_info, sync_groups=False, import_by_
           user.groups.remove(group)
         user.groups.add(*new_groups)
         Group.objects.filter(group__in=remove_groups_filtered).delete()
-    except (AssertionError, LdapSearchException) as e:
+    except (ValidationError, LdapSearchException) as e:
       LOG.warn('Could not import %s: %s' % (ldap_info['username'], e.message))
 
   return imported_users
@@ -926,7 +928,7 @@ def _import_ldap_suboordinate_groups(connection, groupname_pattern, import_membe
               validate_username(ldap_info['username'])
               user = ldap_access.get_ldap_user(username=ldap_info['username'])
               group.user_set.add(user)
-            except AssertionError, e:
+            except ValidationError, e:
               LOG.warn('Could not sync %s: %s' % (ldap_info['username'], e.message))
             except User.DoesNotExist:
               pass
@@ -967,7 +969,7 @@ def _import_ldap_suboordinate_groups(connection, groupname_pattern, import_membe
                 validate_username(ldap_info['username'])
                 user = ldap_access.get_ldap_user(username=ldap_info['username'])
                 group.user_set.add(user)
-              except AssertionError, e:
+              except ValidationError, e:
                 LOG.warn('Could not sync %s: %s' % (ldap_info['username'], e.message))
               except User.DoesNotExist:
                 pass