From cfd7de74ecbc0ff155f75ac850f44b59d3700efd Mon Sep 17 00:00:00 2001 From: Sebastian Wyngaard Date: Mon, 12 Feb 2018 17:32:15 -0500 Subject: [PATCH 1/3] Test GET access control endpoint for a tale --- plugin_tests/tale_test.py | 46 +++++++++++++++++++++++++++++++++++++++ server/rest/tale.py | 13 ++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/plugin_tests/tale_test.py b/plugin_tests/tale_test.py index cc0d6f80..cbd3925f 100644 --- a/plugin_tests/tale_test.py +++ b/plugin_tests/tale_test.py @@ -187,6 +187,52 @@ def testTaleFlow(self): continue self.assertEqual(resp.json[key], tale[key]) + def testTaleAccess(self): + with httmock.HTTMock(mockReposRequest, mockCommitRequest, + mockOtherRequest): + # Grab the default user folders + resp = self.request( + path='/folder', method='GET', user=self.user, params={ + 'parentType': 'user', + 'parentId': self.user['_id'], + 'sort': 'title', + 'sortdir': 1 + }) + folder = resp.json[1] + # Create a new tale from a user image + resp = self.request( + path='/tale', method='POST', user=self.user, + type='application/json', + body=json.dumps( + { + 'imageId': str(self.image['_id']), + 'folderId': folder['_id'] + }) + ) + self.assertStatusOk(resp) + tale_user_image = resp.json + + from girder.constants import AccessType + + # Retrieve access control list for the newly created image + resp = self.request( + path='/tale/%s/access' % tale_user_image['_id'], method='GET', + user=self.user) + self.assertStatusOk(resp) + result_tale_access = resp.json + expected_tale_access = { + 'users': [{ + 'login': self.user['login'], + 'level': AccessType.ADMIN, + 'id': str(self.user['_id']), + 'flags': [], + 'name': '%s %s' % ( + self.user['firstName'], self.user['lastName'])}], + 'groups': [] + } + self.assertEqual(result_tale_access, expected_tale_access) + + def tearDown(self): self.model('user').remove(self.user) self.model('user').remove(self.admin) diff --git a/server/rest/tale.py b/server/rest/tale.py index e1bdda3f..c6633f79 100644 --- a/server/rest/tale.py +++ b/server/rest/tale.py @@ -4,7 +4,7 @@ from girder.api.docs import addModel from girder.api.describe import Description, autoDescribeRoute from girder.api.rest import Resource, filtermodel, RestException -from girder.constants import AccessType, SortDir +from girder.constants import AccessType, SortDir, TokenScope from ..schema.tale import taleModel @@ -22,6 +22,7 @@ def __init__(self): self.route('PUT', (':id',), self.updateTale) self.route('POST', (), self.createTale) self.route('DELETE', (':id',), self.deleteTale) + self.route('GET', (':id', 'access'), self.getTaleAccess) @access.public @filtermodel(model='tale', plugin='wholetale') @@ -148,3 +149,13 @@ def createTale(self, tale, params): category=tale.get('category', 'science'), published=False ) + + @access.user(scope=TokenScope.DATA_OWN) + @autoDescribeRoute( + Description('Get the access control list for a tale') + .modelParam('id', model='tale', plugin='wholetale', level=AccessType.ADMIN) + .errorResponse('ID was invalid.') + .errorResponse('Admin access was denied for the tale.', 403) + ) + def getTaleAccess(self, tale): + return self.model('tale', 'wholetale').getFullAccessList(tale) From e056255de2dfafc45de8c331ad605aba130d3b02 Mon Sep 17 00:00:00 2001 From: Sebastian Wyngaard Date: Tue, 13 Feb 2018 10:07:38 -0500 Subject: [PATCH 2/3] Test PUT access control endpoint for a tale --- plugin_tests/tale_test.py | 125 +++++++++++++++++++++++++++++++++++++- server/models/tale.py | 44 ++++++++++++++ server/rest/tale.py | 18 ++++++ 3 files changed, 185 insertions(+), 2 deletions(-) diff --git a/plugin_tests/tale_test.py b/plugin_tests/tale_test.py index cbd3925f..049c88fa 100644 --- a/plugin_tests/tale_test.py +++ b/plugin_tests/tale_test.py @@ -3,7 +3,7 @@ import json from tests import base from .tests_helpers import \ - GOOD_REPO, GOOD_COMMIT, \ + GOOD_REPO, GOOD_COMMIT, XPRA_REPO, XPRA_COMMIT, \ mockOtherRequest, mockCommitRequest, mockReposRequest @@ -40,6 +40,12 @@ def setUp(self): self.recipe = self.model('recipe', 'wholetale').createRecipe( GOOD_COMMIT, 'https://github.com/' + GOOD_REPO, creator=self.user, public=True) + recipe_admin = self.model('recipe', 'wholetale').createRecipe( + XPRA_COMMIT, 'https://github.com/' + XPRA_REPO, + creator=self.admin, public=True) + self.image_admin = self.model('image', 'wholetale').createImage( + recipe_admin, XPRA_REPO, name="xpra name", creator=self.admin, + public=True) self.image = self.model('image', 'wholetale').createImage( self.recipe, GOOD_REPO, name="my name", creator=self.user, public=True) @@ -206,11 +212,24 @@ def testTaleAccess(self): body=json.dumps( { 'imageId': str(self.image['_id']), - 'folderId': folder['_id'] + 'folderId': folder['_id'], + 'public': True }) ) self.assertStatusOk(resp) tale_user_image = resp.json + # Create a new tale from an admin image + resp = self.request( + path='/tale', method='POST', user=self.user, + type='application/json', + body=json.dumps( + { + 'imageId': str(self.image_admin['_id']), + 'folderId': folder['_id'] + }) + ) + self.assertStatusOk(resp) + tale_admin_image = resp.json from girder.constants import AccessType @@ -232,6 +251,108 @@ def testTaleAccess(self): } self.assertEqual(result_tale_access, expected_tale_access) + # Update the access control list for the tale by adding the admin + # as a second user + input_tale_access = { + "users": [ + { + "login": self.user['login'], + "level": AccessType.ADMIN, + "id": str(self.user['_id']), + "flags": [], + "name": "%s %s" % (self.user['firstName'], self.user['lastName']) + }, + { + 'login': self.admin['login'], + 'level': AccessType.ADMIN, + 'id': str(self.admin['_id']), + 'flags': [], + 'name': '%s %s' % (self.admin['firstName'], self.admin['lastName']) + }], + "groups": []} + resp = self.request( + path='/tale/%s/access' % tale_user_image['_id'], method='PUT', + user=self.user, params={'access': json.dumps(input_tale_access)}) + self.assertStatusOk(resp) + # Check that the returned access control list for the tale is as expected + result_tale_access = resp.json['access'] + result_image_id = resp.json['imageId'] + expected_tale_access = { + "groups": [], + "users": [ + { + "flags": [], + "id": str(self.user['_id']), + "level": AccessType.ADMIN + }, + { + "flags": [], + "id": str(self.admin['_id']), + "level": AccessType.ADMIN + }, + ] + } + self.assertEqual(result_tale_access, expected_tale_access) + # Check that the access control list propagated to the image that the tale + # was built from + resp = self.request( + path='/image/%s/access' % result_image_id, method='GET', + user=self.user) + self.assertStatusOk(resp) + result_image_access = resp.json + expected_image_access = input_tale_access + self.assertEqual(result_image_access, expected_image_access) + + # Update the access control list of a tale that was generated from an image that the user + # does not have admin access to + input_tale_access = { + "users": [ + { + "login": self.user['login'], + "level": AccessType.ADMIN, + "id": str(self.user['_id']), + "flags": [], + "name": "%s %s" % (self.user['firstName'], self.user['lastName']) + }], + "groups": [] + } + resp = self.request( + path='/tale/%s/access' % tale_admin_image['_id'], method='PUT', + user=self.user, params={'access': json.dumps(input_tale_access)}) + self.assertStatus(resp, 403) + + # Check that the access control list was correctly set for the tale + resp = self.request( + path='/tale/%s/access' % tale_admin_image['_id'], method='GET', + user=self.user) + self.assertStatusOk(resp) + result_tale_access = resp.json + expected_tale_access = input_tale_access + self.assertEqual(result_tale_access, expected_tale_access) + + # Check that the access control list did not propagate to the image + resp = self.request( + path='/image/%s/access' % tale_admin_image['imageId'], method='GET', + user=self.user) + self.assertStatus(resp, 403) + + # Setting the access list with bad json should throw an error + resp = self.request( + path='/tale/%s/access' % tale_user_image['_id'], method='PUT', + user=self.user, params={'access': 'badJSON'}) + self.assertStatus(resp, 400) + + # Change the access to private + resp = self.request( + path='/tale/%s/access' % tale_user_image['_id'], method='PUT', + user=self.user, + params={'access': json.dumps(input_tale_access), 'public': False}) + self.assertStatusOk(resp) + resp = self.request( + path='/tale/%s' % tale_user_image['_id'], method='GET', + user=self.user) + self.assertStatusOk(resp) + self.assertEqual(resp.json['public'], False) def tearDown(self): self.model('user').remove(self.user) diff --git a/server/models/tale.py b/server/models/tale.py index 3120385b..cb5b2e97 100644 --- a/server/models/tale.py +++ b/server/models/tale.py @@ -146,3 +146,47 @@ def updateTale(self, tale): """ tale['updated'] = datetime.datetime.utcnow() return self.save(tale) + + def setAccessList(self, doc, access, save=False, user=None, force=False, + setPublic=None, publicFlags=None): + """ + Overrides AccessControlledModel.setAccessList to encapsulate ACL + functionality for a tale. + + :param doc: the tale to set access settings on + :type doc: girder.models.tale + :param access: The access control list + :type access: dict + :param save: Whether the changes should be saved to the database + :type save: bool + :param user: The current user + :param force: Set this to True to set the flags regardless of the passed in + user's permissions. + :type force: bool + :param setPublic: Pass this if you wish to set the public flag on the + resources being updated. + :type setPublic: bool or None + :param publicFlags: Pass this if you wish to set the public flag list on + resources being updated. + :type publicFlags: flag identifier str, or list/set/tuple of them, + or None + """ + if setPublic is not None: + self.setPublic(doc, setPublic, save=False) + + if publicFlags is not None: + doc = self.setPublicFlags(doc, publicFlags, user=user, save=False, + force=force) + + doc = AccessControlledModel.setAccessList(self, doc, access, + user=user, save=save, force=force) + + image = AccessControlledModel.model('image', 'wholetale').load( + doc['imageId'], user=user, level=AccessType.ADMIN) + + if image: + AccessControlledModel.model('image', 'wholetale').setAccessList( + image, access, user=user, save=save, force=force, + setPublic=setPublic, publicFlags=publicFlags) + + return doc diff --git a/server/rest/tale.py b/server/rest/tale.py index c6633f79..e1807e2a 100644 --- a/server/rest/tale.py +++ b/server/rest/tale.py @@ -23,6 +23,7 @@ def __init__(self): self.route('POST', (), self.createTale) self.route('DELETE', (':id',), self.deleteTale) self.route('GET', (':id', 'access'), self.getTaleAccess) + self.route('PUT', (':id', 'access'), self.updateTaleAccess) @access.public @filtermodel(model='tale', plugin='wholetale') @@ -159,3 +160,20 @@ def createTale(self, tale, params): ) def getTaleAccess(self, tale): return self.model('tale', 'wholetale').getFullAccessList(tale) + + @access.user(scope=TokenScope.DATA_OWN) + @autoDescribeRoute( + Description('Update the access control list for a tale.') + .modelParam('id', model='tale', plugin='wholetale', level=AccessType.ADMIN) + .jsonParam('access', 'The JSON-encoded access control list.', requireObject=True) + .jsonParam('publicFlags', 'JSON list of public access flags.', requireArray=True, + required=False) + .param('public', 'Whether the tale should be publicly visible.', dataType='boolean', + required=False) + .errorResponse('ID was invalid.') + .errorResponse('Admin access was denied for the tale.', 403) + ) + def updateTaleAccess(self, tale, access, publicFlags, public): + user = self.getCurrentUser() + return self.model('tale', 'wholetale').setAccessList( + tale, access, save=True, user=user, setPublic=public, publicFlags=publicFlags) From b2fb43a5ffdfc3959bf0c1022d660d2535b4cdc5 Mon Sep 17 00:00:00 2001 From: Sebastian Wyngaard Date: Tue, 20 Feb 2018 10:33:57 -0500 Subject: [PATCH 3/3] Progagate ACL from the tale to the tale's folder --- plugin_tests/tale_test.py | 13 ++++++++++++- server/models/tale.py | 7 +++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/plugin_tests/tale_test.py b/plugin_tests/tale_test.py index 049c88fa..62574c51 100644 --- a/plugin_tests/tale_test.py +++ b/plugin_tests/tale_test.py @@ -233,7 +233,7 @@ def testTaleAccess(self): from girder.constants import AccessType - # Retrieve access control list for the newly created image + # Retrieve access control list for the newly created tale resp = self.request( path='/tale/%s/access' % tale_user_image['_id'], method='GET', user=self.user) @@ -277,6 +277,7 @@ def testTaleAccess(self): # Check that the returned access control list for the tale is as expected result_tale_access = resp.json['access'] result_image_id = resp.json['imageId'] + result_folder_id = resp.json['folderId'] expected_tale_access = { "groups": [], "users": [ @@ -303,6 +304,16 @@ def testTaleAccess(self): expected_image_access = input_tale_access self.assertEqual(result_image_access, expected_image_access) + # Check that the access control list propagated to the folder that the tale + # is associated with + resp = self.request( + path='/folder/%s/access' % result_folder_id, method='GET', + user=self.user) + self.assertStatusOk(resp) + result_folder_access = resp.json + expected_folder_access = input_tale_access + self.assertEqual(result_folder_access, expected_folder_access) + # Update the access control list of a tale that was generated from an image that the user # does not have admin access to input_tale_access = { diff --git a/server/models/tale.py b/server/models/tale.py index cb5b2e97..380017e8 100644 --- a/server/models/tale.py +++ b/server/models/tale.py @@ -181,6 +181,13 @@ def setAccessList(self, doc, access, save=False, user=None, force=False, doc = AccessControlledModel.setAccessList(self, doc, access, user=user, save=save, force=force) + folder = AccessControlledModel.model('folder').load( + doc['folderId'], user=user, level=AccessType.ADMIN) + + AccessControlledModel.model('folder').setAccessList( + folder, access, user=user, save=save, force=force, + setPublic=setPublic, publicFlags=publicFlags) + image = AccessControlledModel.model('image', 'wholetale').load( doc['imageId'], user=user, level=AccessType.ADMIN)