From 3a68740ff7aa183c256087b125b4c6331fde475b Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Tue, 20 Sep 2016 15:21:26 -0400
Subject: [PATCH] Better handling of namespace validation to fix a number of
 issues

- Fixes a bug which allows for underscores at the beginning of namespaces: Fixes #1849
- Allows dots and dashes for newer Docker clients: Fixes #1188
- Has the UI display better messaging associated with namespace entry
---
 static/css/core-ui.css                     |  6 ++++
 static/css/directives/ui/user-setup.css    | 12 +++++++
 static/directives/namespace-input.html     |  5 +++
 static/directives/signup-form.html         | 13 +++++++-
 static/js/app.js                           |  2 +-
 static/js/directives/ui/namespace-input.js | 32 ++++++++++++++++++
 static/js/pages/new-organization.js        |  7 ++--
 static/partials/new-organization.html      | 17 ++++++----
 test/test_api_usage.py                     |  2 +-
 test/test_util.py                          | 38 +++++++++++++++++++++-
 util/validation.py                         | 13 +++++---
 11 files changed, 126 insertions(+), 21 deletions(-)
 create mode 100644 static/directives/namespace-input.html
 create mode 100644 static/js/directives/ui/namespace-input.js

diff --git a/static/css/core-ui.css b/static/css/core-ui.css
index bd482b37e..e61db79a7 100644
--- a/static/css/core-ui.css
+++ b/static/css/core-ui.css
@@ -1479,6 +1479,12 @@ a:focus {
   font-size: 18px;
 }
 
+.co-alert.thin:after {
+  top: 9px;
+  font-size: 13px;
+  left: 19px;
+}
+
 .co-alert-popin-warning {
   margin-left: 10px;
 }
diff --git a/static/css/directives/ui/user-setup.css b/static/css/directives/ui/user-setup.css
index 02a5bbf49..8d98035cd 100644
--- a/static/css/directives/ui/user-setup.css
+++ b/static/css/directives/ui/user-setup.css
@@ -94,4 +94,16 @@
 
 .user-setup-element .user-footer-links a:last-child:after {
     content: "";
+}
+
+.user-setup-element .expandable {
+    height: 0px;
+    transition: height ease-in-out 250ms;
+    margin-top: 16px;
+    margin-bottom: 10px;
+    overflow: hidden;
+}
+
+.user-setup-element .expandable.expanded {
+    height: 60px;
 }
\ No newline at end of file
diff --git a/static/directives/namespace-input.html b/static/directives/namespace-input.html
new file mode 100644
index 000000000..def68b57f
--- /dev/null
+++ b/static/directives/namespace-input.html
@@ -0,0 +1,5 @@
+<span class="namespace-input-element">
+  <input type="text" class="form-control" placeholder="{{ namespaceTitle }}" ng-model="binding"
+         required autofocus ng-pattern="usernamePattern"
+         name="namespaceField">
+</span>
\ No newline at end of file
diff --git a/static/directives/signup-form.html b/static/directives/signup-form.html
index 584dfd1af..698661d50 100644
--- a/static/directives/signup-form.html
+++ b/static/directives/signup-form.html
@@ -9,7 +9,16 @@
 
   <form class="form-signup" name="signupForm" ng-submit="register()" ng-show="!awaitingConfirmation && !registering">
     <label for="username">Username:</label>
-    <input type="text" class="form-control" placeholder="Requested username" name="username" ng-model="newUser.username" autofocus required ng-pattern="/^[a-z0-9_]{4,30}$/">
+    <span class="namespace-input" binding="newUser.username" is-back-incompat="isBackIncompat" namespace-title="Requested username"></span>
+
+    <div class="expandable" ng-class="{'expanded': isBackIncompat || (!signupForm.namespaceField.$error.required && signupForm.namespaceField.$invalid)}">
+      <div class="co-alert co-alert-warning thin" ng-show="isBackIncompat">
+        Usernames with dots or dashes are incompatible with Docker verion 1.8 or older
+      </div>
+      <div class="co-alert co-alert-danger thin" ng-show="!signupForm.namespaceField.$error.required && signupForm.namespaceField.$invalid">
+        Usernames must be alphanumeric and be at least four characters in length
+      </div>
+    </div>
 
     <label for="email">E-mail address:</label>
     <input type="email" class="form-control" placeholder="Your email address" name="email" ng-model="newUser.email" required>
@@ -20,6 +29,8 @@
     <input type="password" class="form-control" placeholder="Verify your password" ng-model="newUser.repeatPassword"
            match="newUser.password" required
            ng-pattern="/^.{8,}$/">
+
+
     <button id="signupButton"
             class="btn btn-primary btn-block landing-signup-button" ng-disabled="signupForm.$invalid" type="submit"
             analytics-on analytics-event="register">
diff --git a/static/js/app.js b/static/js/app.js
index 7caced473..47dfc0e7d 100644
--- a/static/js/app.js
+++ b/static/js/app.js
@@ -1,6 +1,6 @@
 var TEAM_PATTERN = '^[a-z][a-z0-9]+$';
 var ROBOT_PATTERN = '^[a-z][a-z0-9_]{3,29}$';
-var USER_PATTERN = '^[a-z0-9_]{4,30}$';
+var USERNAME_PATTERN = '^([a-z0-9]+(?:[._-][a-z0-9]+)*){4,30}$';
 
 // Define the pages module.
 quayPages = angular.module('quayPages', [], function(){});
diff --git a/static/js/directives/ui/namespace-input.js b/static/js/directives/ui/namespace-input.js
new file mode 100644
index 000000000..1c0c9f468
--- /dev/null
+++ b/static/js/directives/ui/namespace-input.js
@@ -0,0 +1,32 @@
+/**
+ * An element which displays an input box for creating a namespace.
+ */
+angular.module('quay').directive('namespaceInput', function () {
+  var directiveDefinitionObject = {
+    priority: 0,
+    templateUrl: '/static/directives/namespace-input.html',
+    replace: false,
+    transclude: true,
+    restrict: 'C',
+    scope: {
+      'binding': '=binding',
+      'isBackIncompat': '=isBackIncompat',
+      'namespaceTitle': '@namespaceTitle',
+    },
+    controller: function($scope, $element) {
+      $scope.USERNAME_PATTERN = USERNAME_PATTERN;
+      $scope.usernamePattern = new RegExp(USERNAME_PATTERN);
+
+
+      $scope.$watch('binding', function(binding) {
+        if (!binding) {
+          $scope.isBackIncompat = false;
+          return;
+        }
+
+        $scope.isBackIncompat = (binding.indexOf('-') > 0 || binding.indexOf('.') > 0);
+      })
+    }
+  };
+  return directiveDefinitionObject;
+});
\ No newline at end of file
diff --git a/static/js/pages/new-organization.js b/static/js/pages/new-organization.js
index ffd84f3c5..04cf388ff 100644
--- a/static/js/pages/new-organization.js
+++ b/static/js/pages/new-organization.js
@@ -54,9 +54,9 @@
     };
 
     $scope.createNewOrg = function() {
-      $('#orgName').popover('hide');
-
+      $scope.createError = null;
       $scope.creating = true;
+
       var org = $scope.org;
       var data = {
         'name': org.name,
@@ -96,9 +96,6 @@
       }, function(resp) {
         $scope.creating = false;
         $scope.createError = ApiService.getErrorMessage(resp);
-        $timeout(function() {
-          $('#orgName').popover('show');
-        });
       });
     };
   }
diff --git a/static/partials/new-organization.html b/static/partials/new-organization.html
index 2fefdcfdc..1951c781c 100644
--- a/static/partials/new-organization.html
+++ b/static/partials/new-organization.html
@@ -33,19 +33,22 @@
         <!-- Step 2 -->
         <div ng-show="user && !user.anonymous && !created">
           <div class="step-container">
+            <div class="co-alert co-alert-danger" ng-if="createError">
+            {{ createError }}
+            </div>
+
             <form method="post" name="newOrgForm" id="newOrgForm" ng-submit="createNewOrg()">
               <div class="form-group nested">
                 <label for="orgName">Organization Name</label>
                 <div class="field-row">
                   <span class="field-container">
-                    <input id="orgName" name="orgName" type="text" class="form-control"
-                           placeholder="Organization Name"
-                           ng-model="org.name" required autofocus data-trigger="manual"
-                           data-content="{{ createError }}"
-                           data-placement="bottom" data-container="body" ng-pattern="/^[a-z0-9_]{4,30}$/">
+                    <span class="namespace-input" binding="org.name" is-back-incompat="isBackIncompat" namespace-title="Organization name"></span>
                   </span>
-                  <span class="co-alert co-alert-warning thin" ng-show="!newOrgForm.orgName.$error.required && !newOrgForm.orgName.$valid">
-                      Organization names must match [a-z0-9_]+ and be at least four characters in length.
+                  <span class="co-alert co-alert-warning thin" ng-show="isBackIncompat">
+                    Organization names with dots or dashes are incompatible with Docker verion 1.8 or older
+                  </span>
+                  <span class="co-alert co-alert-danger thin" ng-show="!newOrgForm.namespaceField.$error.required && newOrgForm.namespaceField.$invalid">
+                    Organization names must be alphanumeric and be at least four characters in length
                   </span>
                 </div>
                 <span class="description">This will also be the namespace for your repositories. Must be alphanumeric, all lowercase and at least four characters long.</span>
diff --git a/test/test_api_usage.py b/test/test_api_usage.py
index f9d5e29cf..a18847337 100644
--- a/test/test_api_usage.py
+++ b/test/test_api_usage.py
@@ -551,7 +551,7 @@ class TestCreateNewUser(ApiTestCase):
                                            email='test@example.com'),
                                  expected_code=400)
 
-    self.assertEquals('Invalid username auserName: Username must match expression [a-z0-9_]+',
+    self.assertEquals('Invalid username auserName: Username must match expression ^([a-z0-9]+(?:[._-][a-z0-9]+)*)$',
                       json['detail'])
 
   def test_createuser(self):
diff --git a/test/test_util.py b/test/test_util.py
index 47974bfb2..42724bb61 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -4,12 +4,45 @@ import magic
 from itertools import islice
 from semantic_version import Version, Spec
 
-from util.validation import generate_valid_usernames
+from util.validation import generate_valid_usernames, validate_username
 from util.registry.generatorfile import GeneratorFile
 from util.registry.dockerver import docker_version
 from util import slash_join
 from _pyio import BufferedReader
 
+class TestUsernameValidation(unittest.TestCase):
+  def assertValid(self, username):
+    result, _ = validate_username(username)
+    self.assertTrue(result)
+
+  def assertInvalid(self, username):
+    result, _ = validate_username(username)
+    self.assertFalse(result)
+
+  def test_valid(self):
+    self.assertValid('jake')
+    self.assertValid('ja_ke')
+    self.assertValid('te-st')
+    self.assertValid('te.st')
+
+  def test_properlength(self):
+    self.assertValid('z' * 30)
+
+  def test_tooshort(self):
+    self.assertInvalid('j')
+    self.assertInvalid('ja')
+    self.assertInvalid('jk')
+
+  def test_toolong(self):
+    self.assertInvalid('z' * 31)
+
+  def test_invalids(self):
+    self.assertInvalid('_test')
+    self.assertInvalid('Test')
+    self.assertInvalid('hello world')
+    self.assertInvalid('te---st')
+
+
 class TestGeneratorFile(unittest.TestCase):
   def sample_generator(self):
     yield 'this'
@@ -128,6 +161,9 @@ class TestUsernameGenerator(unittest.TestCase):
     self.assert_generated_output('ja__', 'ja00')
     self.assert_generated_output('jake__', 'jake')
 
+  def test_starting_underscore(self):
+    self.assert_generated_output('_jake', 'jake')
+
   def test_short_names(self):
     self.assert_generated_output('a', 'a000')
     self.assert_generated_output('ab', 'ab00')
diff --git a/util/validation.py b/util/validation.py
index 1f6c5666c..b0e27579b 100644
--- a/util/validation.py
+++ b/util/validation.py
@@ -7,13 +7,15 @@ import anunidecode # Don't listen to pylint's lies. This import is required.
 
 INVALID_PASSWORD_MESSAGE = 'Invalid password, password must be at least ' + \
                            '8 characters and contain no whitespace.'
-INVALID_USERNAME_CHARACTERS = r'[^a-z0-9_]'
 VALID_CHARACTERS = string.digits + string.lowercase
 
 MIN_USERNAME_LENGTH = 4
 MAX_USERNAME_LENGTH = 30
 
 VALID_LABEL_KEY_REGEX = r'^[a-z0-9](([a-z0-9]|[-.](?![.-]))*[a-z0-9])?$'
+VALID_USERNAME_REGEX = r'^([a-z0-9]+(?:[._-][a-z0-9]+)*)$'
+
+INVALID_USERNAME_CHARACTERS = r'[^a-z0-9_]'
 
 
 def validate_label_key(label_key):
@@ -29,9 +31,8 @@ def validate_email(email_address):
 
 def validate_username(username):
   # Based off the restrictions defined in the Docker Registry API spec
-  regex_match = (re.search(INVALID_USERNAME_CHARACTERS, username) is None)
-  if not regex_match:
-    return (False, 'Username must match expression [a-z0-9_]+')
+  if not re.match(VALID_USERNAME_REGEX, username):
+    return (False, 'Username must match expression ' + VALID_USERNAME_REGEX)
 
   length_match = (len(username) >= MIN_USERNAME_LENGTH and len(username) <= MAX_USERNAME_LENGTH)
   if not length_match:
@@ -58,7 +59,6 @@ def _gen_filler_chars(num_filler_chars):
 
 
 def generate_valid_usernames(input_username):
-  # Docker's regex: [a-z0-9]+(?:[._-][a-z0-9]+)*
   normalized = input_username.encode('unidecode', 'ignore').strip().lower()
   prefix = re.sub(INVALID_USERNAME_CHARACTERS, '_', normalized)[:30]
   prefix = re.sub(r'_{2,}', '_', prefix)
@@ -66,6 +66,9 @@ def generate_valid_usernames(input_username):
   if prefix.endswith('_'):
     prefix = prefix[0:len(prefix) - 1]
 
+  while prefix.startswith('_'):
+    prefix = prefix[1:]
+
   num_filler_chars = max(0, MIN_USERNAME_LENGTH - len(prefix))
 
   while num_filler_chars + len(prefix) <= MAX_USERNAME_LENGTH: