diff --git a/data/database.py b/data/database.py index bc9c227b4..189179728 100644 --- a/data/database.py +++ b/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) diff --git a/data/migrations/versions/6c7014e84a5e_add_user_prompt_support.py b/data/migrations/versions/6c7014e84a5e_add_user_prompt_support.py new file mode 100644 index 000000000..3959fafd0 --- /dev/null +++ b/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 ### diff --git a/data/model/user.py b/data/model/user.py index 14069ce32..891b2e77f 100644 --- a/data/model/user.py +++ b/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, diff --git a/data/users/federated.py b/data/users/federated.py index 52f7c15cf..b78f90ae4 100644 --- a/data/users/federated.py +++ b/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: diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 8e4a6b43f..01b58abf2 100644 --- a/endpoints/api/user.py +++ b/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) diff --git a/endpoints/oauthlogin.py b/endpoints/oauthlogin.py index dbb7171f5..8698d6430 100644 --- a/endpoints/oauthlogin.py +++ b/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) diff --git a/endpoints/web.py b/endpoints/web.py index fa04a01db..7e95a2217 100644 --- a/endpoints/web.py +++ b/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(): diff --git a/initdb.py b/initdb.py index 07fa92039..d60378900 100644 --- a/initdb.py +++ b/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 diff --git a/static/css/pages/update-user.css b/static/css/pages/update-user.css new file mode 100644 index 000000000..e5931c9b7 --- /dev/null +++ b/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; +} diff --git a/static/directives/namespace-input.html b/static/directives/namespace-input.html index def68b57f..3c20a6325 100644 --- a/static/directives/namespace-input.html +++ b/static/directives/namespace-input.html @@ -1,5 +1,7 @@ + ng-model-options="{'debounce': 200}" + name="namespaceField" + ng-class="hasExternalError ? 'ng-invalid' : ''"> \ No newline at end of file diff --git a/static/js/app.js b/static/js/app.js index 47dfc0e7d..4a8e98cdf 100644 --- a/static/js/app.js +++ b/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') diff --git a/static/js/directives/ui/namespace-input.js b/static/js/directives/ui/namespace-input.js index 1c0c9f468..422539c12 100644 --- a/static/js/directives/ui/namespace-input.js +++ b/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; diff --git a/static/js/pages/update-user.js b/static/js/pages/update-user.js new file mode 100644 index 000000000..319f55760 --- /dev/null +++ b/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); + }); + } +})(); \ No newline at end of file diff --git a/static/js/services/user-service.js b/static/js/services/user-service.js index b60f97806..c64fdffa1 100644 --- a/static/js/services/user-service.js +++ b/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); } }; diff --git a/static/partials/update-user.html b/static/partials/update-user.html new file mode 100644 index 000000000..1f95479f4 --- /dev/null +++ b/static/partials/update-user.html @@ -0,0 +1,36 @@ +
+ + +
+

Confirm Username

+

+ The username {{ user.username }} was automatically generated to conform to the + Docker CLI guidelines for use as a namespace in . +

+

Please confirm the selected username or enter a different username below:

+
+
+ + + + + Username valid + + + Username already taken + + + Could not check username + + + Usernames must be alphanumeric and be at least four characters in length + + + Note: Usernames with dots or dashes are incompatible with Docker verion 1.8 or older + +
+
\ No newline at end of file diff --git a/test/data/test.db b/test/data/test.db index 522cb90c5..d60a97b3d 100644 Binary files a/test/data/test.db and b/test/data/test.db differ diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 6790961b6..7c8014c73 100644 --- a/test/test_api_usage.py +++ b/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): diff --git a/test/test_ldap.py b/test/test_ldap.py index d60767f40..bb264a2b2 100644 --- a/test/test_ldap.py +++ b/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):