From 53f2a31547d161e9081daa5366ca8cae10013f70 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Fri, 27 Sep 2013 15:53:39 -0400 Subject: [PATCH] Fix some bugs with the permissions API. Prevent the user from removing themelves as admin. --- data/model.py | 56 ++++++++++++++++++++++++++++++++++-------------- endpoints/api.py | 20 +++++++++++------ 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/data/model.py b/data/model.py index 6e7b65e48..42dac1ea1 100644 --- a/data/model.py +++ b/data/model.py @@ -8,6 +8,10 @@ from database import * logger = logging.getLogger(__name__) +class DataModelException(Exception): + pass + + def create_user(username, password, email): pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) new_user = User.create(username=username, password_hash=pw_hash, @@ -130,7 +134,7 @@ def set_image_metadata(image_id, namespace_name, repository_name, Image.image_id == image_id)) if not image_list: - raise RuntimeError('No image with specified id and repository') + raise DataModelException('No image with specified id and repository') fetched = image_list[0] fetched.created = dateutil.parser.parse(created_date_str) @@ -160,7 +164,7 @@ def get_tag_image(namespace_name, repository_name, tag_name): RepositoryTag.name == tag_name)) if not fetched: - raise Image.DoesNotExist('Unable to find image for tag.') + raise DataModelException('Unable to find image for tag.') return fetched[0] @@ -202,23 +206,39 @@ def get_user_repo_permissions(user, repository): RepositoryPermission.repository == repository) -def get_user_reponame_permission(user_obj, namespace_name, repository_name): - repo = Repository.get(Repository.name == repository_name, - Repository.namespace == namespace_name) - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, - RepositoryPermission.repository == repo) - return perm +def user_permission_repo_query(username, namespace_name, repository_name): + selected = RepositoryPermission.select(User, Repository, Role, + RepositoryPermission) + with_user = selected.join(User) + with_role = with_user.switch(RepositoryPermission).join(Role) + with_repo = with_role.switch(RepositoryPermission).join(Repository) + return with_repo.where(Repository.name == repository_name, + Repository.namespace == namespace_name, + User.username == username) -def set_user_repo_permission(user_obj, namespace_name, repository_name, +def get_user_reponame_permission(username, namespace_name, repository_name): + fetched = list(user_permission_repo_query(username, namespace_name, + repository_name)) + if not fetched: + raise DataModelException('User does not have permission for repo.') + + return fetched[0] + + +def set_user_repo_permission(username, namespace_name, repository_name, role_name): + if username == namespace_name: + raise DataModelException('Namespace owner must always be admin.') + + user = User.get(User.username == username) repo = Repository.get(Repository.name == repository_name, Repository.namespace == namespace_name) new_role = Role.get(Role.name == role_name) # Fetch any existing permission for this user on the repo try: - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, + perm = RepositoryPermission.get(RepositoryPermission.user == user, RepositoryPermission.repository == repo) perm.role = new_role perm.save() @@ -228,9 +248,13 @@ def set_user_repo_permission(user_obj, namespace_name, repository_name, role=new_role) return new_perm -def delete_user_permission(user_obj, namespace_name, repository_name): - repo = Repository.get(Repository.name == repository_name, - Repository.namespace == namespace_name) - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, - RepositoryPermission.repository == repo) - perm.delete_instance() +def delete_user_permission(username, namespace_name, repository_name): + if username == namespace_name: + raise DataModelException('Namespace owner must always be admin.') + + fetched = list(user_permission_repo_query(username, namespace_name, + repository_name)) + if not fetched: + raise DataModelException('User does not have permission for repo.') + + fetched[0].delete_instance() diff --git a/endpoints/api.py b/endpoints/api.py index 375b2d791..a925d34ff 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -153,8 +153,7 @@ def get_permissions(namespace, repository, username): (namespace, repository, username)) permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - user = current_user.db_user - perm = model.get_user_reponame_permission(user, namespace, repository) + perm = model.get_user_reponame_permission(username, namespace, repository) return jsonify(role_view(perm)) abort(403) # Permission denied @@ -169,11 +168,15 @@ def change_permissions(namespace, repository, username): if permission.can(): new_permission = request.get_json() - user = current_user.db_user logger.debug('Setting permission to: %s for user %s' % (new_permission['role'], username)) - perm = model.set_user_repo_permission(user, namespace, repository, - new_permission['role']) + + try: + perm = model.set_user_repo_permission(username, namespace, repository, + new_permission['role']) + except model.DataModelException: + logger.warning('User tried to remove themselves as admin.') + abort(409) resp = jsonify(role_view(perm)) if request.method == 'POST': @@ -189,7 +192,12 @@ def change_permissions(namespace, repository, username): def delete_permissions(namespace, repository, username): permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - model.delete_user_permission(current_user.db_user, namespace, repository) + try: + model.delete_user_permission(username, namespace, repository) + except model.DataModelException: + logger.warning('User tried to remove themselves as admin.') + abort(409) + return make_response('Deleted', 204) abort(403) # Permission denied