From 0e54b0501c6d778cba84f46db1912eae63425880 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <jschorr@gmail.com>
Date: Mon, 7 Apr 2014 20:37:02 -0400
Subject: [PATCH] Return the reason a username validation failed and add tests
 to verify we are sending the reason to client

---
 data/model/legacy.py   | 18 +++++++++++-------
 test/test_api_usage.py | 18 ++++++++++++++++++
 util/validation.py     | 14 ++++++++++----
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/data/model/legacy.py b/data/model/legacy.py
index 3cfa6c27f..b6376d7d2 100644
--- a/data/model/legacy.py
+++ b/data/model/legacy.py
@@ -61,8 +61,10 @@ class InvalidBuildTriggerException(DataModelException):
 def create_user(username, password, email, is_organization=False):
   if not validate_email(email):
     raise InvalidEmailAddressException('Invalid email address: %s' % email)
-  if not validate_username(username):
-    raise InvalidUsernameException('Invalid username: %s' % username)
+
+  (username_valid, username_issue) = validate_username(username)
+  if not username_valid:
+    raise InvalidUsernameException('Invalid username %s: %s' % (username, username_issue))
 
   # We allow password none for the federated login case.
   if password is not None and not validate_password(password):
@@ -125,9 +127,10 @@ def create_organization(name, email, creating_user):
 
 
 def create_robot(robot_shortname, parent):
-  if not validate_username(robot_shortname):
-    raise InvalidRobotException('The name for the robot \'%s\' is invalid.' %
-                                robot_shortname)
+  (username_valid, username_issue) = validate_username(robot_shortname)
+  if not username_valid:
+    raise InvalidRobotException('The name for the robot \'%s\' is invalid: %s' %
+                                (robot_shortname, username_issue))
 
   username = format_robot_username(parent.username, robot_shortname)
 
@@ -214,8 +217,9 @@ def convert_user_to_organization(user, admin_user):
 
 
 def create_team(name, org, team_role_name, description=''):
-  if not validate_username(name):
-    raise InvalidTeamException('Invalid team name: %s' % name)
+  (username_valid, username_issue) = validate_username(name)
+  if not username_valid:
+    raise InvalidTeamException('Invalid team name %s: %s' % (name, username_issue))
 
   if not org.organization:
     raise InvalidOrganizationException('User with name %s is not an org.' %
diff --git a/test/test_api_usage.py b/test/test_api_usage.py
index f03a48d87..d6b56cf9d 100644
--- a/test/test_api_usage.py
+++ b/test/test_api_usage.py
@@ -292,6 +292,24 @@ class TestCreateNewUser(ApiTestCase):
                                  expected_code=400)
 
     self.assertEquals('The username already exists', json['message'])
+
+  def test_trycreatetooshort(self):
+    json = self.postJsonResponse(User,
+                                 data=dict(username='a',
+                                           password='password',
+                                           email='test@example.com'),
+                                 expected_code=400)
+
+    self.assertEquals('Invalid username a: Username must be between 4 and 30 characters in length', json['error_description'])
+
+  def test_trycreateregexmismatch(self):
+    json = self.postJsonResponse(User,
+                                 data=dict(username='auserName',
+                                           password='password',
+                                           email='test@example.com'),
+                                 expected_code=400)
+
+    self.assertEquals('Invalid username auserName: Username must match expression [a-z0-9_]+', json['error_description'])
     
   def test_createuser(self):
     data = self.postResponse(User,
diff --git a/util/validation.py b/util/validation.py
index 895767d98..8b5b8400d 100644
--- a/util/validation.py
+++ b/util/validation.py
@@ -10,10 +10,16 @@ def validate_email(email_address):
 
 
 def validate_username(username):
-  # Minimum length of 2, maximum length of 255, no url unsafe characters
-  return (re.search(r'[^a-z0-9_]', username) is None and
-          len(username) >= 4 and
-          len(username) <= 30)
+  # Based off the restrictions defined in the Docker Registry API spec
+  regex_match = (re.search(r'[^a-z0-9_]', username) is None)
+  if not regex_match:
+    return (False, 'Username must match expression [a-z0-9_]+')
+
+  length_match = (len(username) >= 4 and len(username) <= 30)
+  if not length_match:
+    return (False, 'Username must be between 4 and 30 characters in length')
+
+  return (True, '')
 
 
 def validate_password(password):