From 73eb66eac5aba2f5eba79a1e66c19714fb3122cb Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 9 Aug 2016 17:58:33 -0400 Subject: [PATCH] Add support for deleting namespaces (users, organizations) Fixes #102 Fixes #105 --- app.py | 14 +++- buildman/component/buildcomponent.py | 17 ++++- data/database.py | 11 ++- data/model/user.py | 75 ++++++++++++++++++- data/queue.py | 13 +++- endpoints/api/organization.py | 21 +++++- endpoints/api/repository.py | 7 +- endpoints/api/superuser.py | 7 +- endpoints/api/user.py | 19 ++++- static/css/core-ui.css | 9 +++ .../directives/ui/delete-namespace-view.css | 3 + static/directives/cor-confirm-dialog.html | 10 ++- static/directives/cor-progress-bar.html | 4 + static/directives/delete-namespace-view.html | 35 +++++++++ static/js/core-ui.js | 17 +++++ .../directives/ui/billing-management-panel.js | 6 +- .../js/directives/ui/delete-namespace-view.js | 34 +++++++++ static/js/pages/org-view.js | 1 + static/js/services/user-service.js | 67 ++++++++++++++++- static/partials/org-view.html | 5 +- static/partials/user-view.html | 5 +- test/test_api_usage.py | 55 +++++++++++++- test/test_suconfig_api.py | 5 +- 23 files changed, 407 insertions(+), 33 deletions(-) create mode 100644 static/css/directives/ui/delete-namespace-view.css create mode 100644 static/directives/cor-progress-bar.html create mode 100644 static/directives/delete-namespace-view.html create mode 100644 static/js/directives/ui/delete-namespace-view.js diff --git a/app.py b/app.py index 7c6d27e1c..160a10301 100644 --- a/app.py +++ b/app.py @@ -208,11 +208,17 @@ dex_login = DexOAuthConfig(app.config, 'DEX_LOGIN_CONFIG') oauth_apps = [github_login, github_trigger, gitlab_trigger, google_login, dex_login] -image_replication_queue = WorkQueue(app.config['REPLICATION_QUEUE_NAME'], tf) +image_replication_queue = WorkQueue(app.config['REPLICATION_QUEUE_NAME'], tf, has_namespace=False) dockerfile_build_queue = WorkQueue(app.config['DOCKERFILE_BUILD_QUEUE_NAME'], tf, - reporter=BuildMetricQueueReporter(metric_queue)) -notification_queue = WorkQueue(app.config['NOTIFICATION_QUEUE_NAME'], tf) -secscan_notification_queue = WorkQueue(app.config['SECSCAN_NOTIFICATION_QUEUE_NAME'], tf) + reporter=BuildMetricQueueReporter(metric_queue), + has_namespace=True) +notification_queue = WorkQueue(app.config['NOTIFICATION_QUEUE_NAME'], tf, has_namespace=True) +secscan_notification_queue = WorkQueue(app.config['SECSCAN_NOTIFICATION_QUEUE_NAME'], tf, + has_namespace=False) + +all_queues = [image_replication_queue, dockerfile_build_queue, notification_queue, + secscan_notification_queue] + secscan_api = SecurityScannerAPI(app, app.config, storage) # Check for a key in config. If none found, generate a new signing key for Docker V2 manifests. diff --git a/buildman/component/buildcomponent.py b/buildman/component/buildcomponent.py index 4881bfa80..e2422ddcc 100644 --- a/buildman/component/buildcomponent.py +++ b/buildman/component/buildcomponent.py @@ -16,6 +16,7 @@ from buildman.jobutil.workererror import WorkerError from data import model from data.database import BUILD_PHASE +from data.model import InvalidRepositoryBuildException HEARTBEAT_DELTA = datetime.timedelta(seconds=30) BUILD_HEARTBEAT_DELAY = datetime.timedelta(seconds=30) @@ -241,8 +242,13 @@ class BuildComponent(BaseComponent): # Parse and update the phase and the status_dict. The status dictionary contains # the pull/push progress, as well as the current step index. with self._build_status as status_dict: - if self._build_status.set_phase(phase, log_data.get('status_data')): - logger.debug('Build %s has entered a new phase: %s', self.builder_realm, phase) + try: + if self._build_status.set_phase(phase, log_data.get('status_data')): + logger.debug('Build %s has entered a new phase: %s', self.builder_realm, phase) + except InvalidRepositoryBuildException: + build_id = self._current_job.repo_build.uuid + logger.info('Build %s was not found; repo was probably deleted', build_id) + return BuildComponent._process_pushpull_status(status_dict, phase, log_data, self._image_info) @@ -300,7 +306,12 @@ class BuildComponent(BaseComponent): except: pass - self._build_status.set_phase(BUILD_PHASE.COMPLETE) + try: + self._build_status.set_phase(BUILD_PHASE.COMPLETE) + except InvalidRepositoryBuildException: + logger.info('Build %s was not found; repo was probably deleted', build_id) + return + trollius.async(self._build_finished(BuildJobResult.COMPLETE)) # Label the pushed manifests with the build metadata. diff --git a/data/database.py b/data/database.py index e3db10efc..c0bbcd232 100644 --- a/data/database.py +++ b/data/database.py @@ -359,8 +359,15 @@ class User(BaseModel): raise RuntimeError('Non-recursive delete on user.') # These models don't need to use transitive deletes, because the referenced objects - # are cleaned up directly - skip_transitive_deletes = {Image} + # are cleaned up directly in the model. + skip_transitive_deletes = {Image, Repository, Team, RepositoryBuild, ServiceKeyApproval, + RepositoryBuildTrigger, ServiceKey, RepositoryPermission, + TeamMemberInvite, Star, RepositoryAuthorizedEmail, TeamMember, + RepositoryTag, PermissionPrototype, DerivedStorageForImage, + TagManifest, AccessToken, OAuthAccessToken, BlobUpload, + RepositoryNotification, OAuthAuthorizationCode, + RepositoryActionCount, TagManifestLabel} + delete_instance_filtered(self, User, delete_nullable, skip_transitive_deletes) diff --git a/data/model/user.py b/data/model/user.py index a711efda7..0f6bce839 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -9,7 +9,8 @@ from datetime import datetime, timedelta from data.database import (User, LoginService, FederatedLogin, RepositoryPermission, TeamMember, Team, Repository, TupleSelector, TeamRole, Namespace, Visibility, EmailConfirmation, Role, db_for_update, random_string_generator, - UserRegion, ImageStorageLocation) + UserRegion, ImageStorageLocation, QueueItem, TeamMemberInvite, + ServiceKeyApproval, OAuthApplication, RepositoryBuildTrigger) from data.model import (DataModelException, InvalidPasswordException, InvalidRobotException, InvalidUsernameException, InvalidEmailAddressException, TooManyUsersException, TooManyLoginAttemptsException, db_transaction, @@ -657,11 +658,81 @@ def detach_external_login(user, service_name): FederatedLogin.service == service).execute() -def delete_user(user): +def get_solely_admined_organizations(user_obj): + """ Returns the organizations admined solely by the given user. """ + orgs = (User.select() + .where(User.organization == True) + .join(Team) + .join(TeamRole) + .where(TeamRole.name == 'admin') + .switch(Team) + .join(TeamMember) + .where(TeamMember.user == user_obj) + .distinct()) + + # Filter to organizations where the user is the sole admin. + solely_admined = [] + for org in orgs: + admin_user_count = (TeamMember.select() + .join(Team) + .join(TeamRole) + .where(Team.organization == org, TeamRole.name == 'admin') + .switch(TeamMember) + .join(User) + .where(User.robot == False) + .distinct() + .count()) + + if admin_user_count == 1: + solely_admined.append(org) + + return solely_admined + + +def delete_user(user, queues, force=False): + if not force and not user.organization: + # Ensure that the user is not the sole admin for any organizations. If so, then the user + # cannot be deleted before those organizations are deleted or reassigned. + organizations = get_solely_admined_organizations(user) + if len(organizations) > 0: + message = 'Cannot delete %s as you are the only admin for organizations: ' % user.username + for index, org in enumerate(organizations): + if index > 0: + message = message + ', ' + + message = message + org.username + + raise DataModelException(message) + + # Delete all queue items for the user. + for queue in queues: + queue.delete_namespaced_items(user.username) + # Delete any repositories under the user's namespace. for repo in list(Repository.select().where(Repository.namespace_user == user)): repository.purge_repository(user.username, repo.name) + if user.organization: + # Delete the organization's teams. + for team in Team.select().where(Team.organization == user): + team.delete_instance(recursive=True) + + # Delete any OAuth approvals and tokens associated with the user. + for app in OAuthApplication.select().where(OAuthApplication.organization == user): + app.delete_instance(recursive=True) + else: + # Remove the user from any teams in which they are a member. + TeamMember.delete().where(TeamMember.user == user).execute() + + # Delete any repository buildtriggers where the user is the connected user. + triggers = RepositoryBuildTrigger.select().where(RepositoryBuildTrigger.connected_user == user) + for trigger in triggers: + trigger.delete_instance(recursive=True, delete_nullable=False) + + # Null out any service key approvals. We technically lose information here, but its better than + # falling and only occurs if a superuser is being deleted. + ServiceKeyApproval.update(approver=None).where(ServiceKeyApproval.approver == user).execute() + # Delete the user itself. user.delete_instance(recursive=True, delete_nullable=True) diff --git a/data/queue.py b/data/queue.py index b2e6fa976..205e30287 100644 --- a/data/queue.py +++ b/data/queue.py @@ -33,12 +33,14 @@ class BuildMetricQueueReporter(object): class WorkQueue(object): """ Work queue defines methods for interacting with a queue backed by the database. """ def __init__(self, queue_name, transaction_factory, - canonical_name_match_list=None, reporter=None, metric_queue=None): + canonical_name_match_list=None, reporter=None, metric_queue=None, + has_namespace=False): self._queue_name = queue_name self._reporter = reporter self._metric_queue = metric_queue self._transaction_factory = transaction_factory self._currently_processing = False + self._has_namespaced_items = has_namespace if canonical_name_match_list is None: self._canonical_name_match_list = [] @@ -130,6 +132,15 @@ class WorkQueue(object): except QueueItem.DoesNotExist: return False + def delete_namespaced_items(self, namespace, subpath=None): + """ Deletes all items in this queue that exist under the given namespace. """ + if not self._has_namespaced_items: + return False + + subpath_query = '%s/' % subpath if subpath else '' + queue_prefix = '%s/%s/%s%%' % (self._queue_name, namespace, subpath_query) + QueueItem.delete().where(QueueItem.queue_name ** queue_prefix).execute() + def put(self, canonical_name_list, message, available_after=0, retries_remaining=5): """ Put an item, if it shouldn't be processed for some number of seconds, diff --git a/endpoints/api/organization.py b/endpoints/api/organization.py index 685094d87..710f84a60 100644 --- a/endpoints/api/organization.py +++ b/endpoints/api/organization.py @@ -6,10 +6,10 @@ from flask import request import features -from app import billing as stripe, avatar +from app import billing as stripe, avatar, all_queues from endpoints.api import (resource, nickname, ApiResource, validate_json_request, request_error, related_user_resource, internal_only, require_user_admin, log_action, - show_if, path_param, require_scope) + show_if, path_param, require_scope, require_fresh_login) from endpoints.exception import Unauthorized, NotFound from endpoints.api.user import User, PrivateRepositories from auth.permissions import (AdministerOrganizationPermission, OrganizationMemberPermission, @@ -199,6 +199,23 @@ class Organization(ApiResource): raise Unauthorized() + @require_scope(scopes.ORG_ADMIN) + @require_fresh_login + @nickname('deleteOrganization') + def delete(self, orgname): + """ Deletes the specified organization. """ + permission = AdministerOrganizationPermission(orgname) + if permission.can(): + try: + org = model.organization.get_organization(orgname) + except model.InvalidOrganizationException: + raise NotFound() + + model.user.delete_user(org, all_queues) + + return 'Deleted', 204 + + @resource('/v1/organization//private') @path_param('orgname', 'The name of the organization') @internal_only diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 6bc29fcfd..9557cb0e5 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -8,8 +8,8 @@ from datetime import timedelta, datetime from flask import request, abort +from app import dockerfile_build_queue from data import model -from data.database import Repository as RepositoryTable from endpoints.api import (truthy_bool, format_date, nickname, log_action, validate_json_request, require_repo_read, require_repo_write, require_repo_admin, RepositoryParamResource, resource, query_param, parse_args, ApiResource, @@ -353,9 +353,14 @@ class Repository(RepositoryParamResource): """ Delete a repository. """ model.repository.purge_repository(namespace, repository) user = model.user.get_namespace_user(namespace) + if features.BILLING: plan = get_namespace_plan(namespace) check_repository_usage(user, plan) + + # Remove any builds from the queue. + dockerfile_build_queue.delete_namespaced_items(namespace, repository) + log_action('delete_repo', namespace, {'repo': repository, 'namespace': namespace}) return 'Deleted', 204 diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index f34604796..477e3d7d6 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -11,7 +11,8 @@ from flask import request, make_response, jsonify import features -from app import app, avatar, superusers, authentication, config_provider, license_validator +from app import (app, avatar, superusers, authentication, config_provider, license_validator, + all_queues) from auth import scopes from auth.auth_context import get_authenticated_user from auth.permissions import SuperUserPermission @@ -366,7 +367,7 @@ class SuperUserManagement(ApiResource): if superusers.is_superuser(username): abort(403) - model.user.delete_user(user) + model.user.delete_user(user, all_queues, force=True) return 'Deleted', 204 abort(403) @@ -500,7 +501,7 @@ class SuperUserOrganizationManagement(ApiResource): if SuperUserPermission().can(): org = model.organization.get_organization(name) - model.user.delete_user(org) + model.user.delete_user(org, all_queues) return 'Deleted', 204 abort(403) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index c6c75c0be..1d09e1be0 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -10,7 +10,7 @@ from peewee import IntegrityError import features -from app import app, billing as stripe, authentication, avatar, user_analytics +from app import app, billing as stripe, authentication, avatar, user_analytics, all_queues from auth import scopes from auth.auth_context import get_authenticated_user from auth.permissions import (AdministerOrganizationPermission, CreateRepositoryPermission, @@ -346,9 +346,11 @@ class User(ApiResource): @validate_json_request('NewUser') def post(self): """ Create a new user. """ + if app.config['AUTHENTICATION_TYPE'] != 'Database': + abort(404) + user_data = request.get_json() invite_code = user_data.get('invite_code', '') - existing_user = model.user.get_nonrobot_user(user_data['username']) if existing_user: raise request_error(message='The username already exists') @@ -373,6 +375,19 @@ class User(ApiResource): except model.user.DataModelException as ex: raise request_error(exception=ex) + @require_user_admin + @require_fresh_login + @nickname('deleteCurrentUser') + @internal_only + def delete(self): + """ Deletes the current user. """ + if app.config['AUTHENTICATION_TYPE'] != 'Database': + abort(404) + + model.user.delete_user(get_authenticated_user(), all_queues) + return 'Deleted', 204 + + @resource('/v1/user/private') @internal_only @show_if(features.BILLING) diff --git a/static/css/core-ui.css b/static/css/core-ui.css index 934aba627..1f256c9b2 100644 --- a/static/css/core-ui.css +++ b/static/css/core-ui.css @@ -1650,4 +1650,13 @@ a:focus { overflow-y: auto; overflow-x: hidden; max-height: 400px; +} + +.cor-confirm-dialog-element .modal-body { + padding: 20px; +} + +.cor-confirm-dialog-element .progress-message { + margin-bottom: 10px; + font-size: 16px; } \ No newline at end of file diff --git a/static/css/directives/ui/delete-namespace-view.css b/static/css/directives/ui/delete-namespace-view.css new file mode 100644 index 000000000..8cf79d3e4 --- /dev/null +++ b/static/css/directives/ui/delete-namespace-view.css @@ -0,0 +1,3 @@ +.delete-namespace-view-element .yellow { + color: #FCA657; +} \ No newline at end of file diff --git a/static/directives/cor-confirm-dialog.html b/static/directives/cor-confirm-dialog.html index 59256c0cf..20f993384 100644 --- a/static/directives/cor-confirm-dialog.html +++ b/static/directives/cor-confirm-dialog.html @@ -8,13 +8,19 @@

Billing Information

-
+
@@ -129,7 +131,6 @@ -
+ +

Billing Information

-
+
diff --git a/test/test_api_usage.py b/test/test_api_usage.py index a18847337..06f350ed9 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -21,7 +21,7 @@ from mockldap import MockLdap from endpoints.api import api_bp, api from endpoints.building import PreparedBuild from endpoints.webhooks import webhooks -from app import app, config_provider, notification_queue +from app import app, config_provider, all_queues, dockerfile_build_queue, notification_queue from buildtrigger.basehandler import BuildTriggerHandler from initdb import setup_database_for_testing, finished_database_for_testing from data import database, model @@ -581,7 +581,6 @@ class TestCreateNewUser(ApiTestCase): teamname='owners')) self.assertNotInTeam(json, NEW_USER_DETAILS['username']) - def test_createuser_withteaminvite_differentemails(self): inviter = model.user.get_user(ADMIN_ACCESS_USER) team = model.team.get_organization_team(ORGANIZATION, 'owners') @@ -606,6 +605,32 @@ class TestCreateNewUser(ApiTestCase): self.assertNotInTeam(json, NEW_USER_DETAILS['username']) +class TestDeleteNamespace(ApiTestCase): + def test_deletenamespaces(self): + self.login(ADMIN_ACCESS_USER) + + # Try to first delete the user. Since they are the sole admin of two orgs, it should fail. + with check_transitive_deletes(): + self.deleteResponse(User, expected_code=400) + + # Delete the two orgs, checking in between. + with check_transitive_deletes(): + self.deleteResponse(Organization, params=dict(orgname=ORGANIZATION), expected_code=204) + self.deleteResponse(User, expected_code=400) # Should still fail. + self.deleteResponse(Organization, params=dict(orgname='library'), expected_code=204) + + # Add some queue items for the user. + notification_queue.put([ADMIN_ACCESS_USER, 'somerepo', 'somename'], '{}') + dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'anotherrepo'], '{}') + + # Now delete the user. + with check_transitive_deletes(): + self.deleteResponse(User, expected_code=204) + + # Ensure the queue items are gone. + self.assertIsNone(notification_queue.get()) + self.assertIsNone(dockerfile_build_queue.get()) + class TestSignin(ApiTestCase): def test_signin_unicode(self): @@ -1798,13 +1823,37 @@ class TestDeleteRepository(ApiTestCase): self.getResponse(Repository, params=dict(repository=self.SIMPLE_REPO)) + # Add a build queue item for the repo. + dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'simple'], '{}') + + # Delete the repository. self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO)) + # Ensure the queue item is gone. + self.assertIsNone(dockerfile_build_queue.get()) + # Verify the repo was deleted. self.getResponse(Repository, params=dict(repository=self.SIMPLE_REPO), expected_code=404) + def test_verify_queue_removal(self): + self.login(ADMIN_ACCESS_USER) + + # Verify the repo exists. + self.getResponse(Repository, + params=dict(repository=self.SIMPLE_REPO)) + + # Add a build queue item for the repo and another repo. + dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'simple'], '{}') + dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'anotherrepo'], '{}') + + # Delete the repository. + self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO)) + + # Ensure the other queue item is still present. + self.assertIsNotNone(dockerfile_build_queue.get()) + def test_deleterepo2(self): self.login(ADMIN_ACCESS_USER) @@ -3717,7 +3766,7 @@ class TestSuperUserCreateInitialSuperUser(ApiTestCase): # Delete all users in the DB. for user in list(database.User.select()): - user.delete_instance(recursive=True) + model.user.delete_user(user, all_queues, force=True) # Create the superuser. self.postJsonResponse(SuperUserCreateInitialSuperUser, data=data) diff --git a/test/test_suconfig_api.py b/test/test_suconfig_api.py index 494866e80..4166c7104 100644 --- a/test/test_suconfig_api.py +++ b/test/test_suconfig_api.py @@ -1,8 +1,9 @@ from test.test_api_usage import ApiTestCase, READ_ACCESS_USER, ADMIN_ACCESS_USER from endpoints.api.suconfig import (SuperUserRegistryStatus, SuperUserConfig, SuperUserConfigFile, SuperUserCreateInitialSuperUser, SuperUserConfigValidate) -from app import config_provider +from app import config_provider, all_queues from data.database import User +from data import model import unittest @@ -95,7 +96,7 @@ class TestSuperUserCreateInitialSuperUser(ApiTestCase): # Delete all the users in the DB. for user in list(User.select()): - user.delete_instance(recursive=True) + model.user.delete_user(user, all_queues, force=True) # This method should now succeed. data = dict(username='cooluser', password='password', email='fake@example.com')