diff --git a/pillar/api/projects/__init__.py b/pillar/api/projects/__init__.py index 808a83e0..ff502f5a 100644 --- a/pillar/api/projects/__init__.py +++ b/pillar/api/projects/__init__.py @@ -3,13 +3,13 @@ from .routes import blueprint_api def setup_app(app, api_prefix): + from . import patch + patch.setup_app(app) + app.on_replace_projects += hooks.override_is_private_field app.on_replace_projects += hooks.before_edit_check_permissions app.on_replace_projects += hooks.protect_sensitive_fields - app.on_replaced_projects += hooks.after_undelete_project - app.on_updated_projects += hooks.after_undelete_project - app.on_update_projects += hooks.override_is_private_field app.on_update_projects += hooks.before_edit_check_permissions app.on_update_projects += hooks.protect_sensitive_fields diff --git a/pillar/api/projects/hooks.py b/pillar/api/projects/hooks.py index 4a269c80..d7d7659e 100644 --- a/pillar/api/projects/hooks.py +++ b/pillar/api/projects/hooks.py @@ -1,8 +1,6 @@ import copy -import datetime import logging -import bson.tz_util from flask import request, abort from pillar import current_app @@ -13,7 +11,7 @@ from pillar.api.node_types.group_texture import node_type_group_texture from pillar.api.node_types.texture import node_type_texture from pillar.api.file_storage_backends import default_storage_backend from pillar.api.utils import authorization, authentication -from pillar.api.utils import remove_private_keys, random_etag +from pillar.api.utils import remove_private_keys from pillar.api.utils.authorization import user_has_role, check_permissions from pillar.auth import current_user from .utils import abort_with_error @@ -84,37 +82,6 @@ def after_delete_project(project: dict): log.warning('Unable to delete files of project %s: %s', pid, r) -def after_undelete_project(project: dict, original: dict): - """Undelete the files that belong to this project. - - We cannot do this via Eve, as it doesn't support PATCHing collections, so - direct MongoDB modification is used to set _deleted=False and provide - new _etag and _updated values. - """ - - if not original: - return - - was_deleted = original.get('_deleted', False) - now_deleted = project.get('_deleted', False) - if not was_deleted or now_deleted: - # This is not an undelete. - return - - pid = project['_id'] - new_etag = random_etag() - now = datetime.datetime.now(tz=bson.tz_util.utc) - - files_coll = current_app.db('files') - update_result = files_coll.update_many( - {'project': pid}, - {'$set': {'_deleted': False, - '_etag': new_etag, - '_updated': now}}) - log.info('undeleted %d of %d file documents of project %s', - update_result.modified_count, update_result.matched_count, pid) - - def protect_sensitive_fields(document, original): """When not logged in as admin, prevents update to certain fields.""" diff --git a/pillar/api/projects/patch.py b/pillar/api/projects/patch.py new file mode 100644 index 00000000..e523363e --- /dev/null +++ b/pillar/api/projects/patch.py @@ -0,0 +1,88 @@ +"""Project patching support.""" + +import datetime +import logging + +import bson.tz_util +import flask +from flask import Blueprint, request +import werkzeug.exceptions as wz_exceptions + +from pillar import current_app +from pillar.auth import current_user +from pillar.api.utils import random_etag, str2id +from pillar.api.utils import authorization + +log = logging.getLogger(__name__) +blueprint = Blueprint('projects.patch', __name__) + + +@blueprint.route('/', methods=['PATCH']) +@authorization.require_login() +def patch_project(project_id: str): + """Undelete a project. + + This is done via a custom PATCH due to the lack of transactions of MongoDB; + we cannot undelete both project-referenced files and file-referenced + projects in one atomic operation. + """ + + # Parse the request + pid = str2id(project_id) + patch = request.get_json() + if not patch: + raise wz_exceptions.BadRequest('Expected JSON body') + + log.debug('User %s wants to PATCH project %s: %s', current_user, pid, patch) + + # 'undelete' is the only operation we support now, so no fancy handler registration. + op = patch.get('op', '') + if op != 'undelete': + log.warning('User %s sent unsupported PATCH op %r to project %s: %s', + current_user, op, pid, patch) + raise wz_exceptions.BadRequest(f'unsupported operation {op!r}') + + # Get the project to find the user's permissions. + proj_coll = current_app.db('projects') + proj = proj_coll.find_one({'_id': pid}) + if not proj: + raise wz_exceptions.NotFound(f'project {pid} not found') + allowed = authorization.compute_allowed_methods('projects', proj) + if 'PUT' not in allowed: + log.warning('User %s tried to undelete project %s but only has permissions %r', + current_user, pid, allowed) + raise wz_exceptions.Forbidden(f'no PUT access to project {pid}') + + if not proj.get('_deleted', False): + raise wz_exceptions.BadRequest(f'project {pid} was not deleted, unable to undelete') + + # Undelete the files. We cannot do this via Eve, as it doesn't support + # PATCHing collections, so direct MongoDB modification is used to set + # _deleted=False and provide new _etag and _updated values. + new_etag = random_etag() + now = datetime.datetime.now(tz=bson.tz_util.utc) + + log.debug('undeleting files before undeleting project %s', pid) + files_coll = current_app.db('files') + update_result = files_coll.update_many( + {'project': pid}, + {'$set': {'_deleted': False, + '_etag': new_etag, + '_updated': now}}) + log.info('undeleted %d of %d file documents of project %s', + update_result.modified_count, update_result.matched_count, pid) + + log.info('undeleting project %s on behalf of user %s', pid, current_user) + update_result = proj_coll.update_one({'_id': pid}, + {'$set': {'_deleted': False}}) + log.info('undeleted %d project document %s', update_result.modified_count, pid) + + resp = flask.Response('', status=204) + resp.location = flask.url_for('projects.view', project_url=proj['url']) + return resp + + +def setup_app(app): + # This needs to be on the same URL prefix as Eve uses for the collection, + # and not /p as used for the other Projects API calls. + app.register_api_blueprint(blueprint, url_prefix='/projects') diff --git a/pillar/web/projects/routes.py b/pillar/web/projects/routes.py index 20b64873..ec09743d 100644 --- a/pillar/web/projects/routes.py +++ b/pillar/web/projects/routes.py @@ -861,37 +861,3 @@ def edit_extension(project: Project, extension_name): return ext.project_settings(project, ext_pages=find_extension_pages()) - - -@blueprint.route('/undelete', methods=['POST']) -@login_required -def undelete(): - """Undelete a deleted project. - - Can only be done by the owner of the project or an admin. - """ - # This function takes an API-style approach, even though it's a web - # endpoint. Undeleting via a REST approach would mean GETting the - # deleted project, which now causes a 404 exception to bubble to the - # client. - from pillar.api.utils import mongo, remove_private_keys - from pillar.api.utils.authorization import check_permissions - - project_id = request.form.get('project_id') - if not project_id: - raise wz_exceptions.BadRequest('missing project ID') - - # Check that the user has PUT permissions on the project itself. - project = mongo.find_one_or_404('projects', project_id) - check_permissions('projects', project, 'PUT') - - pid = project['_id'] - log.info('Undeleting project %s on behalf of %s', pid, current_user.email) - r, _, _, status = current_app.put_internal('projects', remove_private_keys(project), _id=pid) - if status != 200: - log.warning('Error %d un-deleting project %s: %s', status, pid, r) - return 'Error un-deleting project', 500 - - resp = flask.Response('', status=204) - resp.location = flask.url_for('projects.view', project_url=project['url']) - return resp diff --git a/src/templates/projects/index_dashboard.pug b/src/templates/projects/index_dashboard.pug index c721a4f5..48c9008c 100644 --- a/src/templates/projects/index_dashboard.pug +++ b/src/templates/projects/index_dashboard.pug @@ -327,9 +327,16 @@ script. hopToTop(); // Display jump to top button }); + + var patch_url = '{{ url_for('projects.patch.patch_project', project_id='PROJECTID') }}'; function undelete_project(project_id) { console.log('undeleting project', project_id); - $.post('{{ url_for('projects.undelete') }}', {project_id: project_id}) + $.ajax({ + url: patch_url.replace('PROJECTID', project_id), + method: 'PATCH', + data: JSON.stringify({'op': 'undelete'}), + contentType: 'application/json' + }) .done(function(data, textStatus, jqXHR) { location.href = jqXHR.getResponseHeader('Location'); }) diff --git a/tests/test_api/test_project_management.py b/tests/test_api/test_project_management.py index feee7073..001888a0 100644 --- a/tests/test_api/test_project_management.py +++ b/tests/test_api/test_project_management.py @@ -398,42 +398,67 @@ class ProjectEditTest(AbstractProjectTest): self.assertGreater(db_file_after['_updated'], db_file_before['_updated']) self.assertNotEqual(db_file_after['_etag'], db_file_before['_etag']) - def test_undelete_with_put__files_too(self): - from pillar.api.utils import remove_private_keys + def _create_delete_project(self): + """Create and then delete a project.""" + from pillar.api.utils import remove_private_keys # Create test project with a file. project_info = self._create_user_and_project(['subscriber']) project_id = project_info['_id'] - - fid, _ = self.ensure_file_exists({'project': ObjectId(project_id)}) - - # DELETE by owner should also soft-delete the file documents. proj_url = f'/api/projects/{project_id}' - self.delete(proj_url, auth_token='token', etag=project_info['_etag'], - expected_status=204) + etag = project_info['_etag'] - resp = self.get(proj_url, auth_token='token', expected_status=404) + # Assign the file as picture_header so that we have a nice circular reference. + fid, _ = self.ensure_file_exists({'project': ObjectId(project_id)}) + project_info['picture_header'] = str(fid) + resp = self.put(proj_url, auth_token='token', etag=etag, + json=remove_private_keys(project_info)) etag = resp.json()['_etag'] + # DELETE the project. + self.delete(proj_url, auth_token='token', etag=etag, expected_status=204) + with self.app.app_context(): files_coll = self.app.db('files') now = datetime.datetime.now(tz=bson.tz_util.utc) - datetime.timedelta(seconds=5) db_file_before = files_coll.find_one_and_update({'_id': fid}, {'$set': {'_updated': now}}, return_document=ReturnDocument.AFTER) + return db_file_before, fid, proj_url - # PUT on the project should also restore the files. - self.put(proj_url, auth_token='token', etag=etag, - json=remove_private_keys(project_info)) + def test_undelete_with_patch(self): + db_file_before, fid, proj_url = self._create_delete_project() + + # PATCH on the project should also restore the files. + self.patch(proj_url, auth_token='token', json={'op': 'undelete'}, expected_status=204) resp = self.get(f'/api/files/{fid}') self.assertEqual(str(fid), resp.json()['_id']) with self.app.app_context(): + files_coll = self.app.db('files') db_file_after = files_coll.find_one(fid) self.assertGreater(db_file_after['_updated'], db_file_before['_updated']) self.assertNotEqual(db_file_after['_etag'], db_file_before['_etag']) + def test_undelete_other_user(self): + _, fid, proj_url = self._create_delete_project() + + self.create_user(user_id='baddf00dbaddf00dbaddf00d', token='baduser') + + # PATCH on the project should be denied. + self.patch(proj_url, auth_token='baduser', json={'op': 'undelete'}, expected_status=403) + + self.get(f'/api/files/{fid}', expected_status=404) + + with self.app.app_context(): + files_coll = self.app.db('files') + db_file_after = files_coll.find_one(fid) + self.assertTrue(db_file_after['_deleted']) + + resp = self.get(proj_url, auth_token='token', expected_status=404).json() + self.assertTrue(resp['_deleted']) + class ProjectNodeAccess(AbstractProjectTest): def setUp(self, **kwargs):