From dc5af7496c8978f7e782007c67550c44e6941ae2 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 11 May 2015 17:13:42 -0400 Subject: [PATCH] Allow superusers to disable user accounts --- CHANGELOG.md | 1 + auth/auth.py | 8 ++ auth/auth_context.py | 6 + data/database.py | 1 + ...e_add_enabled_column_to_the_user_system.py | 26 ++++ data/model/legacy.py | 32 ++++- data/users.py | 9 +- endpoints/api/superuser.py | 8 +- endpoints/api/user.py | 1 + initdb.py | 6 + static/css/pages/superuser.css | 25 ++++ static/css/quay.css | 22 --- static/directives/signin-form.html | 12 +- static/js/directives/ui/signin-form.js | 5 + static/js/pages/superuser.js | 26 ++++ static/partials/super-user.html | 8 +- test/data/test.db | Bin 778240 -> 778240 bytes test/test_auth.py | 130 ++++++++++++++++++ util/http.py | 2 +- 19 files changed, 291 insertions(+), 37 deletions(-) create mode 100644 data/migrations/versions/154f2befdfbe_add_enabled_column_to_the_user_system.py create mode 100644 static/css/pages/superuser.css create mode 100644 test/test_auth.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2825800e9..5c8d40f5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ The following are features that have been merged, but not yet deployed: +- Add ability to disable users via the superuser panel (#26) - Add a Changelog view to the superuser panel (#186) ### 1.9.7 diff --git a/auth/auth.py b/auth/auth.py index 9ec76483c..21ca79748 100644 --- a/auth/auth.py +++ b/auth/auth.py @@ -35,6 +35,10 @@ def _load_user_from_cookie(): logger.debug('Loading user from cookie: %s', current_user.get_id()) db_user = current_user.db_user() if db_user is not None: + # Don't allow disabled users to login. + if not db_user.enabled: + return None + set_authenticated_user(db_user) loaded = QuayDeferredPermissionUser.for_user(db_user) identity_changed.send(app, identity=loaded) @@ -62,6 +66,10 @@ def _validate_and_apply_oauth_token(token): abort(401, message='OAuth access token has expired: %(token)s', issue='invalid-oauth-token', token=token, headers=authenticate_header) + # Don't allow disabled users to login. + if not validated.authorized_user.enabled: + return None + # We have a valid token scope_set = scopes.scopes_from_scope_string(validated.scope) logger.debug('Successfully validated oauth access token: %s with scope: %s', token, diff --git a/auth/auth_context.py b/auth/auth_context.py index d4ae381be..6125bf275 100644 --- a/auth/auth_context.py +++ b/auth/auth_context.py @@ -17,6 +17,9 @@ def get_authenticated_user(): logger.debug('Loading deferred authenticated user.') loaded = model.get_user_by_uuid(user_uuid) + if not loaded.enabled: + return None + set_authenticated_user(loaded) user = loaded @@ -26,6 +29,9 @@ def get_authenticated_user(): def set_authenticated_user(user_or_robot): + if not user_or_robot.enabled: + raise Exception('Attempt to authenticate a disabled user/robot: %s' % user_or_robot.username) + ctx = _request_ctx_stack.top ctx.authenticated_user = user_or_robot diff --git a/data/database.py b/data/database.py index 648b5058c..b1ea970fa 100644 --- a/data/database.py +++ b/data/database.py @@ -193,6 +193,7 @@ class User(BaseModel): invalid_login_attempts = IntegerField(default=0) last_invalid_login = DateTimeField(default=datetime.utcnow) removed_tag_expiration_s = IntegerField(default=1209600) # Two weeks + enabled = BooleanField(default=True) def delete_instance(self, recursive=False, delete_nullable=False): # If we are deleting a robot account, only execute the subset of queries necessary. diff --git a/data/migrations/versions/154f2befdfbe_add_enabled_column_to_the_user_system.py b/data/migrations/versions/154f2befdfbe_add_enabled_column_to_the_user_system.py new file mode 100644 index 000000000..665eefd47 --- /dev/null +++ b/data/migrations/versions/154f2befdfbe_add_enabled_column_to_the_user_system.py @@ -0,0 +1,26 @@ +"""Add enabled column to the user system + +Revision ID: 154f2befdfbe +Revises: 41f4587c84ae +Create Date: 2015-05-11 17:02:43.507847 + +""" + +# revision identifiers, used by Alembic. +revision = '154f2befdfbe' +down_revision = '41f4587c84ae' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(tables): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('enabled', sa.Boolean(), nullable=False, default=True)) + ### end Alembic commands ### + + +def downgrade(tables): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('user', 'enabled') + ### end Alembic commands ### diff --git a/data/model/legacy.py b/data/model/legacy.py index 4ae304cfb..68b50370c 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -67,6 +67,7 @@ class InvalidRobotException(DataModelException): class InvalidTeamException(DataModelException): pass + class InvalidTeamMemberException(DataModelException): pass @@ -259,16 +260,35 @@ def lookup_robot(robot_username): return found[0] def verify_robot(robot_username, password): - joined = User.select().join(FederatedLogin).join(LoginService) - found = list(joined.where(FederatedLogin.service_ident == password, - LoginService.name == 'quayrobot', - User.username == robot_username)) - if not found: + result = parse_robot_username(robot_username) + if result is None: + raise InvalidRobotException('%s is an invalid robot name' % robot_username) + + # Find the matching robot. + query = (User.select() + .join(FederatedLogin) + .join(LoginService) + .where(FederatedLogin.service_ident == password, + LoginService.name == 'quayrobot', + User.username == robot_username)) + + try: + robot = query.get() + except User.DoesNotExist: msg = ('Could not find robot with username: %s and supplied password.' % robot_username) raise InvalidRobotException(msg) - return found[0] + # Find the owner user and ensure it is not disabled. + try: + owner = User.get(User.username == result[0]) + except User.DoesNotExist: + raise InvalidRobotException('Robot %s owner does not exist' % robot_username) + + if not owner.enabled: + raise InvalidRobotException('This user has been disabled. Please contact your administrator.') + + return robot def regenerate_robot_token(robot_shortname, parent): robot_username = format_robot_username(parent.username, robot_shortname) diff --git a/data/users.py b/data/users.py index 2e93e072b..c5fe926e9 100644 --- a/data/users.py +++ b/data/users.py @@ -410,7 +410,14 @@ class UserAuthentication(object): else: password = decrypted - return self.state.verify_user(username_or_email, password) + (result, err_msg) = self.state.verify_user(username_or_email, password) + if not result: + return (result, err_msg) + + if not result.enabled: + return (None, 'This user has been disabled. Please contact your administrator.') + + return (result, err_msg) def __getattr__(self, name): return getattr(self.state, name, None) diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index bd00c0fe6..46857343a 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -115,7 +115,8 @@ def user_view(user): 'email': user.email, 'verified': user.verified, 'avatar': avatar.get_data_for_user(user), - 'super_user': superusers.is_superuser(user.username) + 'super_user': superusers.is_superuser(user.username), + 'enabled': user.enabled } @resource('/v1/superuser/changelog/') @@ -335,6 +336,11 @@ class SuperUserManagement(ApiResource): if 'email' in user_data: model.update_email(user, user_data['email'], auto_verify=True) + if 'enabled' in user_data: + # Disable/enable the user. + user.enabled = bool(user_data['enabled']) + user.save() + return user_view(user) abort(403) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 6a0567963..585b75eda 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -403,6 +403,7 @@ def conduct_signin(username_or_email, password): return { 'needsEmailVerification': needs_email_verification, 'invalidCredentials': invalid_credentials, + 'message': error_message }, 403 diff --git a/initdb.py b/initdb.py index 7f632ce66..5b29a42a3 100644 --- a/initdb.py +++ b/initdb.py @@ -330,6 +330,12 @@ def populate_database(): new_user_1.stripe_id = TEST_STRIPE_ID new_user_1.save() + disabled_user = model.create_user('disabled', 'password', + 'jschorr+disabled@devtable.com') + disabled_user.verified = True + disabled_user.enabled = False + disabled_user.save() + dtrobot = model.create_robot('dtrobot', new_user_1) new_user_2 = model.create_user('public', 'password', diff --git a/static/css/pages/superuser.css b/static/css/pages/superuser.css new file mode 100644 index 000000000..380e31879 --- /dev/null +++ b/static/css/pages/superuser.css @@ -0,0 +1,25 @@ +.super-user .user-row { + border-bottom: 0px; +} + +.super-user .user-row td { + vertical-align: middle; +} + +.super-user .user-row .user-class { + text-transform: uppercase; +} + +.super-user .user-row .labels { + float: right; + white-space: nowrap; +} + +.super-user .user-row .labels .label { + text-transform: uppercase; + margin-right: 10px; +} + +.super-user .user-row.disabled .avatar { + -webkit-filter: grayscale(100%); +} \ No newline at end of file diff --git a/static/css/quay.css b/static/css/quay.css index 9cabe975a..ca3c0109a 100644 --- a/static/css/quay.css +++ b/static/css/quay.css @@ -3928,28 +3928,6 @@ pre.command:before { padding: 6px; } -.user-row { - border-bottom: 0px; -} - -.user-row td { - vertical-align: middle; -} - -.user-row .user-class { - text-transform: uppercase; -} - -.user-row .labels { - float: right; - white-space: nowrap; -} - -.user-row .labels .label { - text-transform: uppercase; - margin-right: 10px; -} - .form-change input { margin-top: 12px; margin-bottom: 12px; diff --git a/static/directives/signin-form.html b/static/directives/signin-form.html index f7d415cd8..fc5727828 100644 --- a/static/directives/signin-form.html +++ b/static/directives/signin-form.html @@ -1,12 +1,12 @@ -
- +