Add support for temp usernames and an interstitial to confirm username

When a user now logs in for the first time for any external auth (LDAP, JWT, Keystone, Github, Google, Dex), they will be presented with a confirmation screen that affords them the opportunity to change their Quay-assigned username.

Addresses most of the user issues around #74

data/database.py

@@ -372,6 +372,22 @@ class User(BaseModel):
 Namespace = User.alias()
 
 
+class UserPromptKind(BaseModel):
+  name = CharField(index=True)
+
+
+class UserPrompt(BaseModel):
+  user = QuayUserField(allows_robots=False, index=True)
+  kind = ForeignKeyField(UserPromptKind)
+
+  class Meta:
+    database = db
+    read_slaves = (read_slave,)
+    indexes = (
+     (('user', 'kind'), True),
+    )
+
+
 class TeamRole(BaseModel):
   name = CharField(index=True)
 

data/migrations/versions/6c7014e84a5e_add_user_prompt_support.py

@@ -0,0 +1,47 @@
+"""Add user prompt support
+
+Revision ID: 6c7014e84a5e
+Revises: c156deb8845d
+Create Date: 2016-10-31 16:26:31.447705
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '6c7014e84a5e'
+down_revision = 'c156deb8845d'
+
+from alembic import op
+import sqlalchemy as sa
+
+def upgrade(tables):
+    ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('userpromptkind',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('name', sa.String(length=255), nullable=False),
+    sa.PrimaryKeyConstraint('id', name=op.f('pk_userpromptkind'))
+    )
+    op.create_index('userpromptkind_name', 'userpromptkind', ['name'], unique=False)
+    op.create_table('userprompt',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('user_id', sa.Integer(), nullable=False),
+    sa.Column('kind_id', sa.Integer(), nullable=False),
+    sa.ForeignKeyConstraint(['kind_id'], ['userpromptkind.id'], name=op.f('fk_userprompt_kind_id_userpromptkind')),
+    sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_userprompt_user_id_user')),
+    sa.PrimaryKeyConstraint('id', name=op.f('pk_userprompt'))
+    )
+    op.create_index('userprompt_kind_id', 'userprompt', ['kind_id'], unique=False)
+    op.create_index('userprompt_user_id', 'userprompt', ['user_id'], unique=False)
+    op.create_index('userprompt_user_id_kind_id', 'userprompt', ['user_id', 'kind_id'], unique=True)
+    ### end Alembic commands ###
+
+    op.bulk_insert(tables.userpromptkind,
+    [
+        {'name':'confirm_username'},
+    ])
+
+
+def downgrade(tables):
+    ### commands auto generated by Alembic - please adjust! ###
+    op.drop_table('userprompt')
+    op.drop_table('userpromptkind')
+    ### end Alembic commands ###

data/model/user.py

@@ -11,7 +11,8 @@ from data.database import (User, LoginService, FederatedLogin, RepositoryPermiss
                            Team, Repository, TupleSelector, TeamRole, Namespace, Visibility,
                            EmailConfirmation, Role, db_for_update, random_string_generator,
                            UserRegion, ImageStorageLocation, QueueItem, TeamMemberInvite,
-                           ServiceKeyApproval, OAuthApplication, RepositoryBuildTrigger)
+                           ServiceKeyApproval, OAuthApplication, RepositoryBuildTrigger,
+                           UserPromptKind, UserPrompt)
 from data.model import (DataModelException, InvalidPasswordException, InvalidRobotException,
                         InvalidUsernameException, InvalidEmailAddressException,
                         TooManyLoginAttemptsException, db_transaction,
@@ -102,6 +103,38 @@ def change_password(user, new_password):
   notification.delete_notifications_by_kind(user, 'password_required')
 
 
+def has_user_prompts(user):
+  try:
+    UserPrompt.select().where(UserPrompt.user == user).get()
+    return True
+  except UserPrompt.DoesNotExist:
+    return False
+
+def has_user_prompt(user, prompt_name):
+  prompt_kind = UserPromptKind.get(name=prompt_name)
+
+  try:
+    UserPrompt.get(user=user, kind=prompt_kind)
+    return True
+  except UserPrompt.DoesNotExist:
+    return False
+
+
+def create_user_prompt(user, prompt_name):
+  prompt_kind = UserPromptKind.get(name=prompt_name)
+  return UserPrompt.create(user=user, kind=prompt_kind)
+
+
+def remove_user_prompt(user, prompt_name):
+  prompt_kind = UserPromptKind.get(name=prompt_name)
+  UserPrompt.delete().where(UserPrompt.user == user, UserPrompt.kind == prompt_kind).execute()
+
+
+def get_user_prompts(user):
+  query = UserPrompt.select().where(UserPrompt.user == user).join(UserPromptKind)
+  return [prompt.kind.name for prompt in query]
+
+
 def change_username(user_id, new_username):
   (username_valid, username_issue) = validate_username(new_username)
   if not username_valid:
@@ -121,6 +154,10 @@ def change_username(user_id, new_username):
     # Rename the user
     user.username = new_username
     user.save()
+
+    # Remove any prompts for username.
+    remove_user_prompt(user, 'confirm_username')
+
     return user
 
 
@@ -305,11 +342,15 @@ def list_entity_robot_permission_teams(entity_name, include_permissions=False):
 
 
 def create_federated_user(username, email, service_name, service_ident,
-                          set_password_notification, metadata={}, email_required=True):
+                          set_password_notification, metadata={},
+                          email_required=True, confirm_username=False):
   new_user = create_user_noverify(username, email, email_required=email_required)
   new_user.verified = True
   new_user.save()
 
+  if confirm_username:
+    create_user_prompt(new_user, 'confirm_username')
+
   service = LoginService.get(LoginService.name == service_name)
   FederatedLogin.create(user=new_user, service=service,
                         service_ident=service_ident,

data/users/federated.py

@@ -52,7 +52,8 @@ class FederatedUsers(object):
       db_user = model.user.create_federated_user(valid_username, email, self._federated_service,
                                                  username,
                                                  set_password_notification=False,
-                                                 email_required=self._requires_email)
+                                                 email_required=self._requires_email,
+                                                 confirm_username=True)
     else:
       # Update the db attributes from the federated service.
       if email:

endpoints/api/user.py

@@ -66,7 +66,7 @@ def handle_invite_code(invite_code, user):
   return True
 
 
-def user_view(user):
+def user_view(user, previous_username=None):
   def org_view(o, user_admin=True):
     admin_org = AdministerOrganizationPermission(o.username)
     org_response = {
@@ -105,7 +105,7 @@ def user_view(user):
     'avatar': avatar.get_data_for_user(user),
   }
 
-  user_admin = UserAdminPermission(user.username)
+  user_admin = UserAdminPermission(previous_username if previous_username else user.username)
   if user_admin.can():
     user_response.update({
       'can_create_repo': True,
@@ -117,6 +117,7 @@ def user_view(user):
       'invoice_email_address': user.invoice_email_address,
       'preferred_namespace': not (user.stripe_id is None),
       'tag_expiration': user.removed_tag_expiration_s,
+      'prompts': model.user.get_user_prompts(user),
     })
 
     analytics_metadata = user_analytics.get_user_analytics_metadata(user)
@@ -217,7 +218,7 @@ class User(ApiResource):
     'UserView': {
       'type': 'object',
       'description': 'Describes a user',
-      'required': ['verified', 'anonymous', 'avatar'],
+      'required': ['anonymous', 'avatar'],
       'properties': {
         'verified': {
           'type': 'boolean',
@@ -283,6 +284,7 @@ class User(ApiResource):
     """ Update a users details such as password or email. """
     user = get_authenticated_user()
     user_data = request.get_json()
+    previous_username = None
 
     try:
       if 'password' in user_data:
@@ -324,19 +326,29 @@ class User(ApiResource):
         else:
           model.user.update_email(user, new_email, auto_verify=not features.MAILING)
 
-      if ('username' in user_data and user_data['username'] != user.username and
-          features.USER_RENAME):
-        new_username = user_data['username']
-        if model.user.get_user_or_org(new_username) is not None:
-          # Username already used
-          raise request_error(message='Username is already in use')
+      # Check for username rename. A username can be renamed if the feature is enabled OR the user
+      # currently has a confirm_username prompt.
+      if 'username' in user_data:
+        confirm_username = model.user.has_user_prompt(user, 'confirm_username')
+        new_username = user_data.get('username')
+        previous_username = user.username
 
-        model.user.change_username(user.id, new_username)
+        rename_allowed = features.USER_RENAME or confirm_username
+        username_changing = new_username and new_username != previous_username
+
+        if rename_allowed and username_changing:
+          if model.user.get_user_or_org(new_username) is not None:
+            # Username already used.
+            raise request_error(message='Username is already in use')
+
+          user = model.user.change_username(user.id, new_username)
+        elif confirm_username:
+          model.user.remove_user_prompt(user, 'confirm_username')
 
     except model.user.InvalidPasswordException, ex:
       raise request_error(exception=ex)
 
-    return user_view(user)
+    return user_view(user, previous_username=previous_username)
 
   @show_if(features.USER_CREATION)
   @show_if(features.DIRECT_LOGIN)

endpoints/oauthlogin.py

@@ -37,11 +37,11 @@ def get_user(service, token):
     'access_token': token,
     'alt': 'json',
   }
-  get_user = client.get(service.user_endpoint(), params=token_param)
-  if get_user.status_code != requests.codes.ok:
+  got_user = client.get(service.user_endpoint(), params=token_param)
+  if got_user.status_code != requests.codes.ok:
     return {}
 
-  return get_user.json()
+  return got_user.json()
 
 
 def conduct_oauth_login(service, user_id, username, email, metadata={}):
@@ -65,7 +65,7 @@ def conduct_oauth_login(service, user_id, username, email, metadata={}):
 
       to_login = model.user.create_federated_user(new_username, email, service_name.lower(),
                                                   user_id, set_password_notification=True,
-                                                  metadata=metadata)
+                                                  metadata=metadata, confirm_username=True)
 
       # Success, tell analytics
       analytics.track(to_login.username, 'register', {'service': service_name.lower()})
@@ -75,7 +75,7 @@ def conduct_oauth_login(service, user_id, username, email, metadata={}):
         logger.debug('Aliasing with state: %s', state)
         analytics.alias(to_login.username, state)
 
-    except model.InvalidEmailAddressException as ieex:
+    except model.InvalidEmailAddressException:
       message = "The e-mail address %s is already associated " % (email, )
       message = message + "with an existing %s account." % (app.config['REGISTRY_TITLE_SHORT'], )
       message = message + "\nPlease log in with your username and password and "
@@ -87,7 +87,10 @@ def conduct_oauth_login(service, user_id, username, email, metadata={}):
       return render_ologin_error(service_name, ex.message)
 
   if common_login(to_login):
-    return redirect(url_for('web.index'))
+    if model.user.has_user_prompts(to_login):
+      return redirect(url_for('web.updateuser'))
+    else:
+      return redirect(url_for('web.index'))
 
   return render_ologin_error(service_name)
 

endpoints/web.py

@@ -170,6 +170,12 @@ def new():
   return index('')
 
 
+@web.route('/updateuser')
+@no_cache
+def updateuser():
+  return index('')
+
+
 @web.route('/confirminvite')
 @no_cache
 def confirm_invite():

initdb.py

@@ -19,7 +19,7 @@ from data.database import (db, all_models, Role, TeamRole, Visibility, LoginServ
                            ImageStorageTransformation, ImageStorageSignatureKind,
                            ExternalNotificationEvent, ExternalNotificationMethod, NotificationKind,
                            QuayRegion, QuayService, UserRegion, OAuthAuthorizationCode,
-                           ServiceKeyApprovalType, MediaType, LabelSourceType)
+                           ServiceKeyApprovalType, MediaType, LabelSourceType, UserPromptKind)
 from data import model
 from data.queue import WorkQueue
 from app import app, storage as store, tf
@@ -396,6 +396,8 @@ def initialize_database():
   LabelSourceType.create(name='api', mutable=True)
   LabelSourceType.create(name='internal')
 
+  UserPromptKind.create(name='confirm_username')
+
 
 def wipe_database():
   logger.debug('Wiping all data from the DB.')
@@ -781,6 +783,8 @@ def populate_database(minimal=False, with_storage=False):
   fake_queue = WorkQueue('fakequeue', tf)
   fake_queue.put(['canonical', 'job', 'name'], '{}')
 
+  model.user.create_user_prompt(new_user_5, 'confirm_username')
+
   while repositoryactioncounter.count_repository_actions():
     pass
 

static/css/pages/update-user.css

@@ -0,0 +1,25 @@
+.update-user p {
+    margin-top: 20px;
+}
+
+.update-user .username-status {
+    margin-left: 10px;
+    font-size: 125%;
+    vertical-align: middle;
+}
+
+.update-user .username-status .fa {
+    margin-right: 4px;
+}
+
+.update-user  i.fa-exclamation-triangle {
+  color: #FCA657;
+}
+
+.update-user  i.fa-check-circle {
+  color: #2FC98E;
+}
+
+.update-user  i.fa-ban {
+  color: #D64456;
+}

static/directives/namespace-input.html

@@ -1,5 +1,7 @@
 <span class="namespace-input-element">
   <input type="text" class="form-control" placeholder="{{ namespaceTitle }}" ng-model="binding"
          required autofocus ng-pattern="usernamePattern"
-         name="namespaceField">
+         ng-model-options="{'debounce': 200}"
+         name="namespaceField"
+         ng-class="hasExternalError ? 'ng-invalid' : ''">
 </span>

static/js/app.js

@@ -189,6 +189,9 @@ quayApp.config(['$routeProvider', '$locationProvider', 'pages', function($routeP
     // Privacy
     .route('/privacy', 'privacy')
 
+    // Change username
+    .route('/updateuser', 'update-user')
+
     // Landing Page
     .route('/', 'landing')
 

static/js/directives/ui/namespace-input.js

@@ -11,13 +11,14 @@ angular.module('quay').directive('namespaceInput', function () {
     scope: {
       'binding': '=binding',
       'isBackIncompat': '=isBackIncompat',
+      'hasExternalError': '=?hasExternalError',
+
       '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;

static/js/pages/update-user.js

@@ -0,0 +1,80 @@
+(function() {
+  /**
+   * Update user page.
+   */
+  angular.module('quayPages').config(['pages', function(pages) {
+    pages.create('update-user', 'update-user.html', UpdateUserCtrl, {
+      'title': 'Confirm Username'
+    });
+  }]);
+
+  function UpdateUserCtrl($scope, UserService, $location, ApiService) {
+    $scope.state = 'loading';
+
+    UserService.updateUserIn($scope, function(user) {
+        if (!user.prompts || !user.prompts.length) {
+            $location.path('/');
+            return;
+        }
+
+        $scope.state = 'editing';
+        $scope.username = user.username;
+    });
+
+    var confirmUsername = function(username) {
+        if (username == $scope.user.username) {
+            $scope.state = 'confirmed';
+            return;
+        }
+
+        $scope.state = 'confirming';
+        var params = {
+            'username': username
+        };
+
+        ApiService.getUserInformation(null, params).then(function() {
+          $scope.state = 'existing';
+        }, function(resp) {
+          if (resp.status == 404) {
+            $scope.state = 'confirmed';
+          } else {
+            $scope.state = 'error';
+          }
+        });
+    };
+
+    $scope.updateUsername = function(username) {
+        $scope.state = 'updating';
+        var data = {
+            'username': username
+        };
+
+        ApiService.changeUserDetails(data).then(function() {
+          window.location = '/';
+        }, ApiService.errorDisplay('Could not update username'));
+    };
+
+    $scope.hasPrompt = function(user, prompt_name) {
+      if (!user.prompts) {
+        return false;
+      }
+
+      for (var i = 0; i < user.prompts.length; ++i) {
+        if (user.prompts[i] == prompt_name) {
+          return true;
+        }
+      }
+
+      return false;
+    };
+
+    $scope.$watch('username', function(username) {
+        if (!username) {
+            $scope.state = 'editing';
+            return;
+        }
+
+        confirmUsername(username);
+    });
+  }
+})();

static/js/services/user-service.js

@@ -91,8 +91,14 @@ function(ApiService, CookieService, $rootScope, Config, $location) {
         }
       }
 
+      // If the loaded user has a prompt, redirect them to the update page.
+      if (loadedUser.prompts && loadedUser.prompts.length) {
+        $location.path('/updateuser');
+        return;
+      }
+
       if (opt_callback) {
-        opt_callback();
+        opt_callback(loadedUser);
       }
     };
 

static/partials/update-user.html

@@ -0,0 +1,36 @@
+<div class="cor-loader-inline" ng-if="user.anonymous || state == 'updating'"></div>
+<!-- TODO: Support additional kinds of prompts here -->
+
+<div class="update-user" ng-show="hasPrompt(user, 'confirm_username') && state != 'updating'">
+    <h2>Confirm Username</h2>
+    <p>
+    The username <strong>{{ user.username }}</strong> was automatically generated to conform to the
+    Docker CLI guidelines for use as a namespace in <span class="registry-title"></span>.
+    </p>
+    <p>Please confirm the selected username or enter a different username below:</p>
+    <form name="usernameForm" ng-submit="updateUsername(username)">
+        <div class="namespace-input" binding="username" is-back-incompat="isBackIncompat"
+              namespace-title="Username" style="margin-bottom: 20px;"
+              has-external-error="state == 'existing'"></div>
+
+        <input type="submit" class="btn btn-primary"
+               ng-disabled="usernameForm.$invalid || state != 'confirmed'"
+               value="Confirm Username">
+        <span class="cor-loader-inline" ng-show="state == 'confirming'"></span>
+        <span class="username-status" ng-show="state == 'confirmed' && !isBackIncompat">
+            <i class="fa fa-check-circle"></i> Username valid
+        </span>
+        <span class="username-status" ng-show="state == 'existing'">
+            <i class="fa fa-ban"></i> Username already taken
+        </span>
+        <span class="username-status" ng-show="state == 'error'">
+            <i class="fa fa-exclamation-triangle"></i> Could not check username
+        </span>
+        <span class="username-status" ng-show="state == 'editing' && usernameForm.$invalid">
+            Usernames must be alphanumeric and be at least four characters in length
+        </span>
+        <span class="username-status" ng-show="state == 'confirmed' && isBackIncompat">
+            <i class="fa fa-exclamation-triangle"></i> Note: Usernames with dots or dashes are incompatible with Docker verion 1.8 or older
+        </span>
+    </form>
+</div>

test/test_api_usage.py

@@ -144,6 +144,14 @@ class ApiTestCase(unittest.TestCase):
     with self.app.session_transaction() as sess:
       sess[CSRF_TOKEN_KEY] = token
 
+  @contextmanager
+  def toggleFeature(self, name, enabled):
+    import features
+    previous_value = getattr(features, name)
+    setattr(features, name, enabled)
+    yield
+    setattr(features, name, previous_value)
+
   def getJsonResponse(self, resource_name, params={}, expected_code=200):
     rv = self.app.get(api.url_for(resource_name, **params))
     self.assertEquals(expected_code, rv.status_code)
@@ -523,6 +531,76 @@ class TestChangeUserDetails(ApiTestCase):
                                 data=dict(invoice_email=False))
     self.assertEquals(False, json['invoice_email'])
 
+  def test_changeusername_temp(self):
+    self.login(READ_ACCESS_USER)
+    user = model.user.get_user(READ_ACCESS_USER)
+    model.user.create_user_prompt(user, 'confirm_username')
+    self.assertTrue(model.user.has_user_prompt(user, 'confirm_username'))
+
+    # Add a robot under the user's namespace.
+    model.user.create_robot('somebot', user)
+
+    # Rename the user.
+    json = self.putJsonResponse(User, data=dict(username='someotherusername'))
+
+    # Ensure the username was changed.
+    self.assertEquals('someotherusername', json['username'])
+    self.assertFalse(model.user.has_user_prompt(user, 'confirm_username'))
+
+    # Ensure the robot was changed.
+    self.assertIsNone(model.user.get_user(READ_ACCESS_USER + '+somebot'))
+    self.assertIsNotNone(model.user.get_user('someotherusername+somebot'))
+
+  def test_changeusername_temp_samename(self):
+    self.login(READ_ACCESS_USER)
+    user = model.user.get_user(READ_ACCESS_USER)
+    model.user.create_user_prompt(user, 'confirm_username')
+    self.assertTrue(model.user.has_user_prompt(user, 'confirm_username'))
+
+    json = self.putJsonResponse(User, data=dict(username=READ_ACCESS_USER))
+
+    # Ensure the username was not changed but they are no longer temporarily named.
+    self.assertEquals(READ_ACCESS_USER, json['username'])
+    self.assertFalse(model.user.has_user_prompt(user, 'confirm_username'))
+
+  def test_changeusername_notallowed(self):
+    with self.toggleFeature('USER_RENAME', False):
+      self.login(ADMIN_ACCESS_USER)
+      user = model.user.get_user(ADMIN_ACCESS_USER)
+      self.assertFalse(model.user.has_user_prompt(user, 'confirm_username'))
+
+      json = self.putJsonResponse(User, data=dict(username='someotherusername'))
+      self.assertEquals(ADMIN_ACCESS_USER, json['username'])
+      self.assertTrue('prompts' in json)
+
+      self.assertIsNone(model.user.get_user('someotherusername'))
+      self.assertIsNotNone(model.user.get_user(ADMIN_ACCESS_USER))
+
+  def test_changeusername_allowed(self):
+    with self.toggleFeature('USER_RENAME', True):
+      self.login(ADMIN_ACCESS_USER)
+      user = model.user.get_user(ADMIN_ACCESS_USER)
+      self.assertFalse(model.user.has_user_prompt(user, 'confirm_username'))
+
+      json = self.putJsonResponse(User, data=dict(username='someotherusername'))
+      self.assertEquals('someotherusername', json['username'])
+      self.assertTrue('prompts' in json)
+
+      self.assertIsNotNone(model.user.get_user('someotherusername'))
+      self.assertIsNone(model.user.get_user(ADMIN_ACCESS_USER))
+
+  def test_changeusername_already_used(self):
+    self.login(READ_ACCESS_USER)
+    user = model.user.get_user(READ_ACCESS_USER)
+    model.user.create_user_prompt(user, 'confirm_username')
+    self.assertTrue(model.user.has_user_prompt(user, 'confirm_username'))
+
+    # Try to change to a used username.
+    self.putJsonResponse(User, data=dict(username=ADMIN_ACCESS_USER), expected_code=400)
+
+    # Change to a new username.
+    self.putJsonResponse(User, data=dict(username='unusedusername'))
+
 
 class TestCreateNewUser(ApiTestCase):
   def test_existingusername(self):

test/test_ldap.py

@@ -3,6 +3,7 @@ import unittest
 from app import app
 from initdb import setup_database_for_testing, finished_database_for_testing
 from data.users import LDAPUsers
+from data import model
 from mockldap import MockLdap
 from mock import patch
 
@@ -119,6 +120,7 @@ class TestLDAP(unittest.TestCase):
     # Verify we can login.
     (response, _) = self.ldap.verify_and_link_user('someuser', 'somepass')
     self.assertEquals(response.username, 'someuser')
+    self.assertTrue(model.user.has_user_prompt(response, 'confirm_username'))
 
     # Verify we can confirm the user.
     (response, _) = self.ldap.confirm_existing_user('someuser', 'somepass')
@@ -220,6 +222,7 @@ class TestLDAP(unittest.TestCase):
     result, _ = self.ldap.confirm_existing_user('someuser', 'somepass')
     self.assertIsNotNone(result)
     self.assertEquals('someuser', result.username)
+    self.assertTrue(model.user.has_user_prompt(user, 'confirm_username'))
 
   def test_query(self):
     def initializer(uri, trace_level=0):