diff --git a/api/auth/listauth.py b/api/auth/listauth.py index c121573e313533a085cb4177f5ecb64c4dbbb0e2..19e75d264a269f9e3ec388c2aa9ae27a60b329c1 100644 --- a/api/auth/listauth.py +++ b/api/auth/listauth.py @@ -7,6 +7,7 @@ import sys from .. import config from ..types import Origin +from ..web.errors import APIPermissionException from . import _get_access, INTEGER_PERMISSIONS log = config.log @@ -45,6 +46,7 @@ def files_sublist(handler, container): access = _get_access(handler.uid, container) def g(exec_op): def f(method, _id, query_params=None, payload=None, exclude_params=None): + errors = None if method == 'GET' and container.get('public', False): min_access = -1 elif method == 'GET': @@ -52,16 +54,27 @@ def files_sublist(handler, container): elif method in ['POST', 'PUT']: min_access = INTEGER_PERMISSIONS['rw'] elif method =='DELETE': - min_access = INTEGER_PERMISSIONS['admin'] - if container.get('origin',{}).get('type') in [str(Origin.user), str(Origin.job)]: - min_access = INTEGER_PERMISSIONS['rw'] + min_access = INTEGER_PERMISSIONS['rw'] + file_is_original_data = bool(container.get('origin',{}).get('type') not in [str(Origin.user), str(Origin.job)]) + + if file_is_original_data: + min_access = INTEGER_PERMISSIONS['admin'] + + if file_is_original_data and access == INTEGER_PERMISSIONS['rw']: + # The user was not granted access because the container had original data + errors = {'reason': 'original_data_present'} + else: + errors = {'reason': 'permission_denied'} + else: min_access = sys.maxint + log.warning('the user access is {} and the min access is {}'.format(access, min_access)) + if access >= min_access: return exec_op(method, _id, query_params, payload, exclude_params) else: - handler.abort(403, 'user not authorized to perform a {} operation on the list'.format(method)) + raise APIPermissionException('user not authorized to perform a {} operation on the list'.format(method), errors=errors) return f return g diff --git a/tests/integration_tests/python/test_containers.py b/tests/integration_tests/python/test_containers.py index 695c14b155b800b31263ba236165832f0ed2fb61..9b807de69e96ddacc6c7952ffea6ff48a14db5bf 100644 --- a/tests/integration_tests/python/test_containers.py +++ b/tests/integration_tests/python/test_containers.py @@ -1267,11 +1267,16 @@ def test_container_delete_tag(data_builder, default_payload, as_root, as_admin, assert r.status_code == 403 assert r.json()['reason'] == 'permission_denied' - # try to delete acquisition with perms + # try to delete acquisition without perms r = as_user.delete('/acquisitions/' + acquisition) assert r.status_code == 403 assert r.json()['reason'] == 'permission_denied' + # try to delete file without perms + r = as_user.delete('/acquisitions/' + acquisition + '/files/test2.csv') + assert r.status_code == 403 + assert r.json()['reason'] == 'permission_denied' + # Add user as rw r = as_user.get('/users/self') assert r.ok @@ -1298,6 +1303,11 @@ def test_container_delete_tag(data_builder, default_payload, as_root, as_admin, assert r.status_code == 403 assert r.json()['reason'] == 'original_data_present' + # try to delete "original data" file without admin perms + r = as_user.delete('/acquisitions/' + acquisition + '/files/test2.csv') + assert r.status_code == 403 + assert r.json()['reason'] == 'original_data_present' + # Add session level analysis r = as_admin.post('/sessions/' + session + '/analyses', params={'job': 'true'}, json={ 'analysis': {'label': 'with-job'}, @@ -1319,8 +1329,7 @@ def test_container_delete_tag(data_builder, default_payload, as_root, as_admin, assert r.status_code == 400 # verify that a non-referenced file _can_ be deleted from the same acquisition - assert as_admin.post('/acquisitions/' + acquisition + '/files', files=file_form('unrelated.csv')).ok - assert as_admin.delete('/acquisitions/' + acquisition + '/files/unrelated.csv').ok + assert as_admin.delete('/acquisitions/' + acquisition + '/files/test2.csv').ok # delete collection assert collection in as_admin.get('/acquisitions/' + acquisition).json()['collections'] @@ -1336,10 +1345,6 @@ def test_container_delete_tag(data_builder, default_payload, as_root, as_admin, assert as_admin.get('/sessions/' + session + '/analyses/' + analysis).status_code == 404 assert as_admin.get('/analyses/' + analysis).status_code == 404 - # try to delete acquisition without admin perms - r = as_user.delete('/acquisitions/' + acquisition) - assert r.status_code == 403 - # delete acquisition assert as_admin.delete('/acquisitions/' + acquisition).ok assert 'deleted' in api_db.acquisitions.find_one({'_id': bson.ObjectId(acquisition)})