Refactor password validation

lib/galaxy/model/__init__.py

@@ -51,6 +51,7 @@ import galaxy.security.passwords
 import galaxy.util
 from galaxy.model.item_attrs import get_item_annotation_str, UsesAnnotations
 from galaxy.security import get_permitted_actions
+from galaxy.security.validate_user_input import validate_password_str
 from galaxy.util import (directory_hash_id, ready_name_for_url,
                          unicodify, unique_id)
 from galaxy.util.bunch import Bunch
@@ -387,6 +388,9 @@ class User(Dictifiable, RepresentById):
         """
         Set user password to the digest of `cleartext`.
         """
+        message = validate_password_str(cleartext, cleartext)
+        if message:
+            raise Exception("Invalid password: %s" % message)
         if User.use_pbkdf2:
             self.password = galaxy.security.passwords.hash_password(cleartext)
         else:

lib/galaxy/security/passwords.py

@@ -22,6 +22,8 @@ def hash_password(password):
     """
     Hash a password, currently will use the PBKDF2 scheme.
     """
+    if password is None:
+        raise Exception("password cannot be None")
     return hash_password_PBKDF2(password)
 
 

lib/galaxy/security/validate_user_input.py

@@ -26,17 +26,44 @@ FILL_CHAR = '-'
 PASSWORD_MIN_LEN = 6
 
 
-def validate_email(trans, email, user=None, check_dup=True, allow_empty=False):
-    """
-    Validates the email format, also checks whether the domain is blacklisted in the disposable domains configuration.
-    """
+def validate_email_str(email):
+    """Validates a string containing an email address."""
     message = ''
-    if (user and user.email == email) or (email == "" and allow_empty):
-        return message
     if not(VALID_EMAIL_RE.match(email)):
         message = "The format of the email address is not correct."
     elif len(email) > EMAIL_MAX_LEN:
         message = "Email address cannot be more than %d characters in length." % EMAIL_MAX_LEN
+    return message
+
+
+def validate_password_str(password, confirm):
+    if password != confirm:
+        return "Passwords do not match."
+    if not password or len(password) < PASSWORD_MIN_LEN:
+        return "Use a password of at least %d characters." % PASSWORD_MIN_LEN
+    return ""
+
+
+def validate_publicname_str(publicname):
+    """Validates a string containing a public username."""
+    if len(publicname) < PUBLICNAME_MIN_LEN:
+        return "Public name must be at least %d characters in length." % (PUBLICNAME_MIN_LEN)
+    if len(publicname) > PUBLICNAME_MAX_LEN:
+        return "Public name cannot be more than %d characters in length." % (PUBLICNAME_MAX_LEN)
+    if not(VALID_PUBLICNAME_RE.match(publicname)):
+        return "Public name must contain only lower-case letters, numbers, '.', '_' and '-'."
+    return ''
+
+
+def validate_email(trans, email, user=None, check_dup=True, allow_empty=False):
+    """
+    Validates the email format, also checks whether the domain is blacklisted in the disposable domains configuration.
+    """
+    if (user and user.email == email) or (email == "" and allow_empty):
+        return ''
+    message = validate_email_str(email)
+    if message:
+        pass
     elif check_dup and trans.sa_session.query(trans.app.model.User).filter(func.lower(trans.app.model.User.table.c.email) == email.lower()).first():
         message = "User with email '%s' already exists." % email
     #  If the blacklist is not empty filter out the disposable domains.
@@ -56,12 +83,9 @@ def validate_publicname(trans, publicname, user=None):
     """
     if user and user.username == publicname:
         return ''
-    if len(publicname) < PUBLICNAME_MIN_LEN:
-        return "Public name must be at least %d characters in length." % (PUBLICNAME_MIN_LEN)
-    if len(publicname) > PUBLICNAME_MAX_LEN:
-        return "Public name cannot be more than %d characters in length." % (PUBLICNAME_MAX_LEN)
-    if not(VALID_PUBLICNAME_RE.match(publicname)):
-        return "Public name must contain only lower-case letters, numbers, '.', '_' and '-'."
+    message = validate_publicname_str(publicname)
+    if message:
+        return message
     if trans.sa_session.query(trans.app.model.User).filter_by(username=publicname).first():
         return "Public name is taken; please choose another."
     return ''
@@ -82,8 +106,4 @@ def transform_publicname(publicname):
 
 
 def validate_password(trans, password, confirm):
-    if len(password) < PASSWORD_MIN_LEN:
-        return "Use a password of at least %d characters." % PASSWORD_MIN_LEN
-    if password != confirm:
-        return "Passwords do not match."
-    return ""
+    return validate_password_str(password, confirm)

lib/galaxy/webapps/galaxy/controllers/admin.py

@@ -17,6 +17,7 @@ from galaxy.actions.admin import AdminActions
 from galaxy.exceptions import ActionInputError, MessageException
 from galaxy.model import tool_shed_install as install_model
 from galaxy.tool_shed.util.repository_util import get_ids_of_tool_shed_repositories_being_installed
+from galaxy.security.validate_user_input import validate_password
 from galaxy.util import (
     nice_size,
     sanitize_text,
@@ -1378,10 +1379,9 @@ class AdminGalaxy(controller.JSAppLauncher, AdminActions, UsesQuotaMixin, QuotaP
             else:
                 password = payload.get('password')
                 confirm = payload.get('confirm')
-                if len(password) < 6:
-                    return self.message_exception(trans, 'Use a password of at least 6 characters.')
-                elif password != confirm:
-                    return self.message_exception(trans, 'Passwords do not match.')
+                message = validate_password(trans, password, confirm)
+                if message:
+                    return self.message_exception(trans, message)
                 for user in users.values():
                     user.set_password_cleartext(password)
                     trans.sa_session.add(user)