From 79073b36e29efd28879592bc27e50c9860700ef8 Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Mon, 17 Jul 2017 10:44:38 -0500 Subject: [PATCH 1/5] new flag to propagate group perm changes --- api/handlers/listhandler.py | 21 ++-- .../python/test_propagation.py | 95 +++++++++++++++++++ 2 files changed, 107 insertions(+), 9 deletions(-) diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index f488dae3f..515206c21 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -209,35 +209,38 @@ class PermissionsListHandler(ListHandler): def post(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).post(cont_name, list_name, **kwargs) - self._propagate_project_permissions(cont_name, _id) + self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) return result def put(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).put(cont_name, list_name, **kwargs) - self._propagate_project_permissions(cont_name, _id) + self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) return result def delete(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).delete(cont_name, list_name, **kwargs) - self._propagate_project_permissions(cont_name, _id) + self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) return result - def _propagate_project_permissions(self, cont_name, _id): + def _propagate_permissions(self, cont_name, _id, propagate=False): """ - method to propagate permissions from a project to its sessions and acquisitions + method to propagate permissions from a container/group to its sessions and acquisitions """ - if cont_name == 'projects': + if (cont_name == 'groups' and propagate) or cont_name == 'projects': try: - oid = bson.ObjectId(_id) + if cont_name == 'projects': + oid = bson.ObjectId(_id) + else: + oid = _id update = {'$set': { - 'permissions': config.db.projects.find_one({'_id': oid},{'permissions': 1})['permissions'] + 'permissions': config.db[cont_name].find_one({'_id': oid},{'permissions': 1})['permissions'] }} hierarchy.propagate_changes(cont_name, oid, {}, update) except APIStorageException: - self.abort(500, 'permissions not propagated from project {} to sessions'.format(_id)) + self.abort(500, 'permissions not propagated from {} {} down hierarchy'.format(cont_name, _id)) class NotesListHandler(ListHandler): diff --git a/test/integration_tests/python/test_propagation.py b/test/integration_tests/python/test_propagation.py index 67fa27715..a2792df99 100644 --- a/test/integration_tests/python/test_propagation.py +++ b/test/integration_tests/python/test_propagation.py @@ -185,6 +185,101 @@ def get_user_in_perms(perms, uid): user = get_user_in_perms(perms, user_id) assert r.ok and user is None +# Test group permission propagation +def test_add_and_remove_user_group_permission(data_builder, as_admin): + """ + Tests: + - changing permissions at a group level with flag triggers propagation + - additive change to list propagates properly + - change to list propagates properly + - removal from list propagates properly + """ + def get_user_in_perms(perms, uid): + for perm in perms: + if perm['_id'] == uid: + return perm + return None + + group = data_builder.create_group() + project = data_builder.create_project() + session = data_builder.create_session() + acquisition = data_builder.create_acquisition() + + user_id = 'propagation@user.com' + + # Add user to group permissions + payload = {'_id': user_id, 'access': 'admin'} + r = as_admin.post('/groups/' + group + '/permissions', json=payload, params={'propagate': True}) + assert r.ok + + r = as_admin.get('/groups/' + group) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user + + r = as_admin.get('/projects/' + project) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user + + r = as_admin.get('/sessions/' + session) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user + + r = as_admin.get('/acquisitions/' + acquisition) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user + + # Modify user permissions + payload = {'access': 'rw', '_id': user_id} + r = as_admin.put('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': True}) + assert r.ok + + r = as_admin.get('/groups/' + group) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user and user['access'] == 'rw' + + r = as_admin.get('/projects/' + project) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user and user['access'] == 'rw' + + r = as_admin.get('/sessions/' + session) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user and user['access'] == 'rw' + + r = as_admin.get('/acquisitions/' + acquisition) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user and user['access'] == 'rw' + + # Remove user from project permissions + r = as_admin.delete('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': True}) + assert r.ok + + r = as_admin.get('/groups/' + group) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user is None + + r = as_admin.get('/projects/' + project) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user is None + + r = as_admin.get('/sessions/' + session) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user is None + + r = as_admin.get('/acquisitions/' + acquisition) + perms = r.json()['permissions'] + user = get_user_in_perms(perms, user_id) + assert r.ok and user is None # Test tag pool renaming and deletion def test_add_rename_remove_group_tag(data_builder, as_admin): From 4188e28dbd66c6f69e53933d57d84dc22972b82a Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Tue, 18 Jul 2017 11:24:04 -0500 Subject: [PATCH 2/5] isolated change push --- api/dao/hierarchy.py | 18 +++++++++ api/handlers/listhandler.py | 40 ++++++++++++++----- .../python/test_propagation.py | 7 ++-- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/api/dao/hierarchy.py b/api/dao/hierarchy.py index 44dd38865..b197b9c05 100644 --- a/api/dao/hierarchy.py +++ b/api/dao/hierarchy.py @@ -131,6 +131,24 @@ def get_parent_tree(cont_name, _id): return tree +def propagate_group_change(_id, update, oper): + # Apply change to projects + if oper == 'POST': + config.db.projects.update_many({'group': _id}, {'$addToSet': {'permissions': update}}) + elif oper == 'PUT': + config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) + elif oper == 'DELETE': + config.db.projects.update_many({'group': _id, 'permissions._id' : update}, {'$pull' : {'permissions': {'_id': update}}}) + else: + raise APIStorageException("Cannot propagate {} operation".format(oper)) + + # Apply change to sessions and acquisitions + projects = config.db.projects.find({'group':_id}) + for project in projects: + propagate_changes('projects', project['_id'], {}, {'$set': { + 'permissions': project['permissions'] + }}) + def propagate_changes(cont_name, _id, query, update): """ diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 515206c21..849178802 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -185,7 +185,7 @@ def _initialize_request(self, cont_name, list_name, _id, query_params=None): query_params = None container = storage.get_container(_id, query_params) if container is not None: - if self.superuser_request: + if self.superuser_request or self.user_is_admin: permchecker = always_ok elif self.public_request: permchecker = listauth.public_request(self, container) @@ -209,32 +209,43 @@ class PermissionsListHandler(ListHandler): def post(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).post(cont_name, list_name, **kwargs) - self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) + + if cont_name == 'groups' and self.request.params.get('propagate') =='true': + self._propagate_group_permission(_id, self.request.json_body, oper='POST') + else: + self._propagate_permissions(cont_name, _id) return result def put(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).put(cont_name, list_name, **kwargs) - self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) + payload = self.request.json_body + payload['_id'] = kwargs.get('_id') + if cont_name == 'groups' and self.request.params.get('propagate') =='true': + self._propagate_group_permission(_id, payload, oper='PUT') + else: + self._propagate_permissions(cont_name, _id) return result def delete(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).delete(cont_name, list_name, **kwargs) - self._propagate_permissions(cont_name, _id, self.request.params.get('propagate')) + + if cont_name == 'groups' and self.request.params.get('propagate') =='true': + self._propagate_group_permission(_id, kwargs.get('_id'), oper='DELETE') + else: + self._propagate_permissions(cont_name, _id) return result - def _propagate_permissions(self, cont_name, _id, propagate=False): + def _propagate_permissions(self, cont_name, _id): """ method to propagate permissions from a container/group to its sessions and acquisitions """ - if (cont_name == 'groups' and propagate) or cont_name == 'projects': + + if cont_name == 'projects': try: - if cont_name == 'projects': - oid = bson.ObjectId(_id) - else: - oid = _id + oid = bson.ObjectId(_id) update = {'$set': { 'permissions': config.db[cont_name].find_one({'_id': oid},{'permissions': 1})['permissions'] }} @@ -242,6 +253,15 @@ def _propagate_permissions(self, cont_name, _id, propagate=False): except APIStorageException: self.abort(500, 'permissions not propagated from {} {} down hierarchy'.format(cont_name, _id)) + def _propagate_group_permission(self, _id, update, oper=None): + """ + method to propagate an isolated change to a groups permissions list to its projects + """ + try: + hierarchy.propagate_group_change(_id, update, oper) + except APIStorageException as e: + self.abort(400, e.message) + class NotesListHandler(ListHandler): """ diff --git a/test/integration_tests/python/test_propagation.py b/test/integration_tests/python/test_propagation.py index a2792df99..735cdb9bb 100644 --- a/test/integration_tests/python/test_propagation.py +++ b/test/integration_tests/python/test_propagation.py @@ -209,7 +209,7 @@ def get_user_in_perms(perms, uid): # Add user to group permissions payload = {'_id': user_id, 'access': 'admin'} - r = as_admin.post('/groups/' + group + '/permissions', json=payload, params={'propagate': True}) + r = as_admin.post('/groups/' + group + '/permissions', json=payload, params={'propagate': 'true'}) assert r.ok r = as_admin.get('/groups/' + group) @@ -220,6 +220,7 @@ def get_user_in_perms(perms, uid): r = as_admin.get('/projects/' + project) perms = r.json()['permissions'] user = get_user_in_perms(perms, user_id) + assert r.json()['group'] == group assert r.ok and user r = as_admin.get('/sessions/' + session) @@ -234,7 +235,7 @@ def get_user_in_perms(perms, uid): # Modify user permissions payload = {'access': 'rw', '_id': user_id} - r = as_admin.put('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': True}) + r = as_admin.put('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': 'true'}) assert r.ok r = as_admin.get('/groups/' + group) @@ -258,7 +259,7 @@ def get_user_in_perms(perms, uid): assert r.ok and user and user['access'] == 'rw' # Remove user from project permissions - r = as_admin.delete('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': True}) + r = as_admin.delete('/groups/' + group + '/permissions/' + user_id, json=payload, params={'propagate': 'true'}) assert r.ok r = as_admin.get('/groups/' + group) From 59133f865e6da4f4483b4bc1f3b9281c451dc7f9 Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Thu, 27 Jul 2017 14:17:23 -0500 Subject: [PATCH 3/5] condensed hierarchy functions --- api/dao/hierarchy.py | 41 +++++++++++++++++++------------------ api/handlers/listhandler.py | 26 ++++++++++------------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/api/dao/hierarchy.py b/api/dao/hierarchy.py index b197b9c05..d845ee4e0 100644 --- a/api/dao/hierarchy.py +++ b/api/dao/hierarchy.py @@ -131,33 +131,34 @@ def get_parent_tree(cont_name, _id): return tree -def propagate_group_change(_id, update, oper): - # Apply change to projects - if oper == 'POST': - config.db.projects.update_many({'group': _id}, {'$addToSet': {'permissions': update}}) - elif oper == 'PUT': - config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) - elif oper == 'DELETE': - config.db.projects.update_many({'group': _id, 'permissions._id' : update}, {'$pull' : {'permissions': {'_id': update}}}) - else: - raise APIStorageException("Cannot propagate {} operation".format(oper)) - - # Apply change to sessions and acquisitions - projects = config.db.projects.find({'group':_id}) - for project in projects: - propagate_changes('projects', project['_id'], {}, {'$set': { - 'permissions': project['permissions'] - }}) - -def propagate_changes(cont_name, _id, query, update): +def propagate_changes(cont_name, _id, query, update, oper=None): """ Propagates changes down the heirarchy tree. cont_name and _id refer to top level container (which will not be modified here) """ - if cont_name == 'groups': + # Apply change to projects + if cont_name == 'groups' and oper: + if oper == 'POST': + config.db.projects.update_many({'group': _id}, {'$addToSet': {'permissions': update}}) + elif oper == 'PUT': + config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) + elif oper == 'DELETE': + config.db.projects.update_many({'group': _id, 'permissions._id' : update}, {'$pull' : {'permissions': {'_id': update}}}) + else: + raise APIStorageException("Cannot propagate {} operation".format(oper)) + + # Apply change to sessions and acquisitions + projects = config.db.projects.find({'group':_id}) + for project in projects: + propagate_changes('projects', project['_id'], {}, {'$set': { + 'permissions': project['permissions'] + }}) + + + elif cont_name == 'groups': project_ids = [p['_id'] for p in config.db.projects.find({'group': _id}, [])] session_ids = [s['_id'] for s in config.db.sessions.find({'project': {'$in': project_ids}}, [])] diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 849178802..016c61028 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -211,7 +211,7 @@ def post(self, cont_name, list_name, **kwargs): result = super(PermissionsListHandler, self).post(cont_name, list_name, **kwargs) if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_group_permission(_id, self.request.json_body, oper='POST') + self._propagate_permissions(cont_name, _id, update=self.request.json_body, oper='POST') else: self._propagate_permissions(cont_name, _id) return result @@ -223,7 +223,7 @@ def put(self, cont_name, list_name, **kwargs): payload = self.request.json_body payload['_id'] = kwargs.get('_id') if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_group_permission(_id, payload, oper='PUT') + self._propagate_permissions(cont_name, _id, update=payload, oper='PUT') else: self._propagate_permissions(cont_name, _id) return result @@ -233,17 +233,22 @@ def delete(self, cont_name, list_name, **kwargs): result = super(PermissionsListHandler, self).delete(cont_name, list_name, **kwargs) if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_group_permission(_id, kwargs.get('_id'), oper='DELETE') + self._propagate_permissions(cont_name, _id, update=kwargs.get('_id'), oper='DELETE') else: self._propagate_permissions(cont_name, _id) return result - def _propagate_permissions(self, cont_name, _id): + def _propagate_permissions(self, cont_name, _id, update=None, oper=None): """ method to propagate permissions from a container/group to its sessions and acquisitions """ - - if cont_name == 'projects': + if cont_name == 'groups': + if oper: + try: + hierarchy.propagate_changes(cont_name, _id, {}, update, oper=oper) + except APIStorageException as e: + self.abort(400, e.message) + elif cont_name == 'projects': try: oid = bson.ObjectId(_id) update = {'$set': { @@ -253,15 +258,6 @@ def _propagate_permissions(self, cont_name, _id): except APIStorageException: self.abort(500, 'permissions not propagated from {} {} down hierarchy'.format(cont_name, _id)) - def _propagate_group_permission(self, _id, update, oper=None): - """ - method to propagate an isolated change to a groups permissions list to its projects - """ - try: - hierarchy.propagate_group_change(_id, update, oper) - except APIStorageException as e: - self.abort(400, e.message) - class NotesListHandler(ListHandler): """ From 4e832d7a10c650b90509855aafec535679c9ced3 Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Thu, 27 Jul 2017 15:09:59 -0500 Subject: [PATCH 4/5] post will update current perms --- api/dao/hierarchy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/dao/hierarchy.py b/api/dao/hierarchy.py index d845ee4e0..93b6471e4 100644 --- a/api/dao/hierarchy.py +++ b/api/dao/hierarchy.py @@ -142,7 +142,8 @@ def propagate_changes(cont_name, _id, query, update, oper=None): # Apply change to projects if cont_name == 'groups' and oper: if oper == 'POST': - config.db.projects.update_many({'group': _id}, {'$addToSet': {'permissions': update}}) + config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) + config.db.projects.update_many({'group': _id, 'permissions._id' : {'$ne': update['_id']}}, {'$addToSet': {'permissions': update}}) elif oper == 'PUT': config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) elif oper == 'DELETE': From 3ccb4690e09f5e3574f08bac728df87e1740cd89 Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Fri, 11 Aug 2017 15:10:25 -0500 Subject: [PATCH 5/5] reverted the propagate method --- api/dao/hierarchy.py | 25 ++++--------------------- api/handlers/listhandler.py | 27 +++++++++++++++------------ 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/api/dao/hierarchy.py b/api/dao/hierarchy.py index 93b6471e4..2ba1b7499 100644 --- a/api/dao/hierarchy.py +++ b/api/dao/hierarchy.py @@ -132,34 +132,15 @@ def get_parent_tree(cont_name, _id): return tree -def propagate_changes(cont_name, _id, query, update, oper=None): +def propagate_changes(cont_name, _id, query, update): """ Propagates changes down the heirarchy tree. cont_name and _id refer to top level container (which will not be modified here) """ - # Apply change to projects - if cont_name == 'groups' and oper: - if oper == 'POST': - config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) - config.db.projects.update_many({'group': _id, 'permissions._id' : {'$ne': update['_id']}}, {'$addToSet': {'permissions': update}}) - elif oper == 'PUT': - config.db.projects.update_many({'group': _id, 'permissions._id' : update['_id']}, {'$set': {'permissions.$.access': update['access']}}) - elif oper == 'DELETE': - config.db.projects.update_many({'group': _id, 'permissions._id' : update}, {'$pull' : {'permissions': {'_id': update}}}) - else: - raise APIStorageException("Cannot propagate {} operation".format(oper)) - - # Apply change to sessions and acquisitions - projects = config.db.projects.find({'group':_id}) - for project in projects: - propagate_changes('projects', project['_id'], {}, {'$set': { - 'permissions': project['permissions'] - }}) - - elif cont_name == 'groups': + if cont_name == 'groups': project_ids = [p['_id'] for p in config.db.projects.find({'group': _id}, [])] session_ids = [s['_id'] for s in config.db.sessions.find({'project': {'$in': project_ids}}, [])] @@ -174,6 +155,8 @@ def propagate_changes(cont_name, _id, query, update, oper=None): config.db.sessions.update_many(session_q, update) config.db.acquisitions.update_many(acquisition_q, update) + + # Apply change to projects elif cont_name == 'projects': session_ids = [s['_id'] for s in config.db.sessions.find({'project': _id}, [])] diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 016c61028..282fb347b 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -209,10 +209,12 @@ class PermissionsListHandler(ListHandler): def post(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).post(cont_name, list_name, **kwargs) + payload = self.request.json_body if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_permissions(cont_name, _id, update=self.request.json_body, oper='POST') - else: + self._propagate_permissions(cont_name, _id, query={'permissions._id' : payload['_id']}, update={'$set': {'permissions.$.access': payload['access']}}) + self._propagate_permissions(cont_name, _id, query={'permissions._id': {'$ne': payload['_id']}}, update={'$addToSet': {'permissions': payload}}) + elif cont_name != 'groups': self._propagate_permissions(cont_name, _id) return result @@ -223,8 +225,8 @@ def put(self, cont_name, list_name, **kwargs): payload = self.request.json_body payload['_id'] = kwargs.get('_id') if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_permissions(cont_name, _id, update=payload, oper='PUT') - else: + self._propagate_permissions(cont_name, _id, query={'permissions._id' : payload['_id']}, update={'$set': {'permissions.$.access': payload['access']}}) + elif cont_name != 'groups': self._propagate_permissions(cont_name, _id) return result @@ -233,21 +235,22 @@ def delete(self, cont_name, list_name, **kwargs): result = super(PermissionsListHandler, self).delete(cont_name, list_name, **kwargs) if cont_name == 'groups' and self.request.params.get('propagate') =='true': - self._propagate_permissions(cont_name, _id, update=kwargs.get('_id'), oper='DELETE') - else: + self._propagate_permissions(cont_name, _id, query={'permissions._id' : kwargs.get('_id')}, update={'$pull' : {'permissions': {'_id': kwargs.get('_id')}}}) + elif cont_name != 'groups': self._propagate_permissions(cont_name, _id) return result - def _propagate_permissions(self, cont_name, _id, update=None, oper=None): + def _propagate_permissions(self, cont_name, _id, query=None, update=None): """ method to propagate permissions from a container/group to its sessions and acquisitions """ + if query is None: + query = {} if cont_name == 'groups': - if oper: - try: - hierarchy.propagate_changes(cont_name, _id, {}, update, oper=oper) - except APIStorageException as e: - self.abort(400, e.message) + try: + hierarchy.propagate_changes(cont_name, _id, query, update) + except APIStorageException as e: + self.abort(400, e.message) elif cont_name == 'projects': try: oid = bson.ObjectId(_id)