details: http://www.bx.psu.edu/hg/galaxy/rev/166348341193 changeset: 3271:166348341193 user: Greg Von Kuster greg@bx.psu.edu date: Tue Jan 26 13:26:42 2010 -0500 description: Fix some problems related to setting the 'manage permissions' permissin on library datasets.

diffstat:

lib/galaxy/security/__init__.py | 46 +++++++++++-------------- lib/galaxy/web/controllers/library_common.py | 8 ++-- templates/dataset/edit_attributes.mako | 2 +- templates/library/common/ldda_permissions.mako | 5 +- test/functional/test_security_and_libraries.py | 9 ++-- 5 files changed, 33 insertions(+), 37 deletions(-)

diffs (189 lines):

diff -r 04ad7a4b5c56 -r 166348341193 lib/galaxy/security/__init__.py --- a/lib/galaxy/security/__init__.py Tue Jan 26 10:23:45 2010 -0500 +++ b/lib/galaxy/security/__init__.py Tue Jan 26 13:26:42 2010 -0500 @@ -21,12 +21,12 @@ IN_ACCESSIBLE = 'access_error' ILL_LEGITIMATE = 'legitimate_error' permitted_actions = Bunch( - DATASET_MANAGE_PERMISSIONS = Action( "manage permissions", "Role members can manage the roles associated with this dataset", "grant" ), + DATASET_MANAGE_PERMISSIONS = Action( "manage permissions", "Role members can manage the roles associated with permissions on this dataset", "grant" ), DATASET_ACCESS = Action( "access", "Role members can import this dataset into their history for analysis", "restrict" ), - LIBRARY_ACCESS = Action( "access library", "Restrict access to this library to role members only", "restrict" ), + LIBRARY_ACCESS = Action( "access library", "Restrict access to this library to only role members", "restrict" ), LIBRARY_ADD = Action( "add library item", "Role members can add library items to this library item", "grant" ), LIBRARY_MODIFY = Action( "modify library item", "Role members can modify this library item", "grant" ), - LIBRARY_MANAGE = Action( "manage library permissions", "Role members can manage roles associated with this library item", "grant" ) + LIBRARY_MANAGE = Action( "manage library permissions", "Role members can manage roles associated with permissions on this library item", "grant" ) ) def get_action( self, name, default=None ): """Get a permitted action by its dict key or action name""" @@ -77,7 +77,7 @@ raise "Unimplemented Method" def make_library_public( self, library ): raise "Unimplemented Method" - def get_library_dataset_permissions( self, library_dataset ): + def get_permissions( self, library_dataset ): raise "Unimplemented Method" def check_library_dataset_access( self, trans, library_id, **kwd ): raise "Unimplemented Method" @@ -326,26 +326,25 @@ if dp.action == self.permitted_actions.DATASET_ACCESS.action: self.sa_session.delete( dp ) self.sa_session.flush() - def get_dataset_permissions( self, dataset ): + def get_permissions( self, item ): """ - Return a dictionary containing the actions and associated roles on dataset. + Return a dictionary containing the actions and associated roles on item. The dictionary looks like: { Action : [ Role, Role ] } - dataset must be an instance of Dataset() - """ + """ permissions = {} - for dp in dataset.actions: - action = self.get_action( dp.action ) + for item_permission in item.actions: + action = self.get_action( item_permission.action ) if action in permissions: - permissions[ action ].append( dp.role ) + permissions[ action ].append( item_permission.role ) else: - permissions[ action ] = [ dp.role ] + permissions[ action ] = [ item_permission.role ] return permissions def copy_dataset_permissions( self, src, dst ): if not isinstance( src, self.model.Dataset ): src = src.dataset if not isinstance( dst, self.model.Dataset ): dst = dst.dataset - self.set_all_dataset_permissions( dst, self.get_dataset_permissions( src ) ) + self.set_all_dataset_permissions( dst, self.get_permissions( src ) ) def privately_share_dataset( self, dataset, users = [] ): intersect = None for user in users: @@ -385,6 +384,14 @@ action = action.action for role_assoc in [ permission_class( action, library_item, role ) for role in roles ]: self.sa_session.add( role_assoc ) + if isinstance( library_item, self.model.LibraryDatasetDatasetAssociation ) and \ + action == self.permitted_actions.LIBRARY_MANAGE.action: + # Handle the special case when we are setting the LIBRARY_MANAGE_PERMISSION on a + # library_dataset_dataset_association since the roles need to be applied to the + # DATASET_MANAGE_PERMISSIONS permission on the associated dataset + permissions = {} + permissions[ self.permitted_actions.DATASET_MANAGE_PERMISSIONS ] = roles + self.set_dataset_permission( library_item.dataset, permissions ) self.sa_session.flush() def library_is_public( self, library ): # A library is considered public if there are no "access" actions associated with it. @@ -395,19 +402,6 @@ if lp.action == self.permitted_actions.LIBRARY_ACCESS.action: self.sa_session.delete( lp ) self.sa_session.flush() - def get_library_dataset_permissions( self, library_dataset ): - # Permissions will always be the same for LibraryDatasets and associated - # LibraryDatasetDatasetAssociations - if isinstance( library_dataset, self.model.LibraryDatasetDatasetAssociation ): - library_dataset = library_dataset.library_dataset - permissions = {} - for library_dataset_permission in library_dataset.actions: - action = self.get_action( library_dataset_permission.action ) - if action in permissions: - permissions[ action ].append( library_dataset_permission.role ) - else: - permissions[ action ] = [ library_dataset_permission.role ] - return permissions def check_library_dataset_access( self, trans, library_id, **kwd ): # library_id must be decoded before being sent msg = '' diff -r 04ad7a4b5c56 -r 166348341193 lib/galaxy/web/controllers/library_common.py --- a/lib/galaxy/web/controllers/library_common.py Tue Jan 26 10:23:45 2010 -0500 +++ b/lib/galaxy/web/controllers/library_common.py Tue Jan 26 13:26:42 2010 -0500 @@ -299,6 +299,7 @@ messagetype = "error" return trans.response.send_redirect( web.url_for( controller='library_common', action='folder_permissions', + cntrller=cntrller, id=trans.security.encode_id( folder.id ), library_id=library_id, msg=util.sanitize_text( msg ), @@ -542,12 +543,11 @@ for ldda in lddas: permissions = [] # Check the library level permissions - the permissions on the LibraryDatasetDatasetAssociation - # will always be the same as the permissions on the associated LibraryDataset, so we only need to - # check one Library object - for library_permission in trans.app.security_agent.get_library_dataset_permissions( ldda.library_dataset ): + # will always be the same as the permissions on the associated LibraryDataset. + for library_permission in trans.app.security_agent.get_permissions( ldda.library_dataset ): if library_permission.action not in permissions: permissions.append( library_permission.action ) - for dataset_permission in trans.app.security_agent.get_dataset_permissions( ldda.dataset ): + for dataset_permission in trans.app.security_agent.get_permissions( ldda.dataset ): if dataset_permission.action not in permissions: permissions.append( dataset_permission.action ) permissions.sort() diff -r 04ad7a4b5c56 -r 166348341193 templates/dataset/edit_attributes.mako --- a/templates/dataset/edit_attributes.mako Tue Jan 26 10:23:45 2010 -0500 +++ b/templates/dataset/edit_attributes.mako Tue Jan 26 13:26:42 2010 -0500 @@ -172,7 +172,7 @@ <div class="form-row"> %if data.dataset.actions: <ul> - %for action, roles in trans.app.security_agent.get_dataset_permissions( data.dataset ).items(): + %for action, roles in trans.app.security_agent.get_permissions( data.dataset ).items(): %if roles: <li>${action.description}</li> <ul> diff -r 04ad7a4b5c56 -r 166348341193 templates/library/common/ldda_permissions.mako --- a/templates/library/common/ldda_permissions.mako Tue Jan 26 10:23:45 2010 -0500 +++ b/templates/library/common/ldda_permissions.mako Tue Jan 26 13:26:42 2010 -0500 @@ -57,5 +57,6 @@ %endif

<% ldda_ids = ",".join( [ trans.security.encode_id( d.id ) for d in lddas ] ) %> -## LIBRARY_ACCESS is a special permission that is set only at the library level. -${render_permission_form( lddas[0], name_str, h.url_for( controller='library_common', action='ldda_permissions', cntrller=cntrller, library_id=library_id, folder_id=trans.security.encode_id( lddas[0].library_dataset.folder.id ), id=ldda_ids ), roles, do_not_render=[ 'LIBRARY_ACCESS' ] )} +## LIBRARY_ACCESS is a special permission that is set only at the library level, +## and DATASET_MANAGE_PERMISSIONS is inherited to the dataset from the ldda. +${render_permission_form( lddas[0], name_str, h.url_for( controller='library_common', action='ldda_permissions', cntrller=cntrller, library_id=library_id, folder_id=trans.security.encode_id( lddas[0].library_dataset.folder.id ), id=ldda_ids ), roles, do_not_render=[ 'LIBRARY_ACCESS', 'DATASET_MANAGE_PERMISSIONS' ] )} diff -r 04ad7a4b5c56 -r 166348341193 test/functional/test_security_and_libraries.py --- a/test/functional/test_security_and_libraries.py Tue Jan 26 10:23:45 2010 -0500 +++ b/test/functional/test_security_and_libraries.py Tue Jan 26 13:26:42 2010 -0500 @@ -1216,7 +1216,8 @@ if not dps: raise AssertionError( 'No DatasetPermissionss created for dataset id: %d' % last_dataset_created.id ) if len( dps ) > 1: - raise AssertionError( 'More than 1 DatasetPermissionss created for dataset id: %d' % last_dataset_created.id ) + actions = [ dp.action for dp in dps ] + raise AssertionError( 'More than 1 DatasetPermissionss created for dataset id: %d, permissions: %s' % ( last_dataset_created.id, str( actions ) ) ) for dp in dps: if not dp.action == 'manage permissions': raise AssertionError( 'DatasetPermissions.action "%s" is not the DefaultHistoryPermission setting of "manage permissions"' \ @@ -1427,7 +1428,7 @@ for ldda in latest_3_lddas: ldda_ids += '%s,' % self.security.encode_id( ldda.id ) ldda_ids = ldda_ids.rstrip( ',' ) - permissions = [ 'DATASET_ACCESS', 'DATASET_MANAGE_PERMISSIONS' ] + permissions = [ 'DATASET_ACCESS', 'LIBRARY_MANAGE' ] def build_url( permissions, role ): # We'll bypass the library_admin/datasets method and directly call the library_admin/dataset method, setting # access, manage permissions, and edit metadata permissions to role_one @@ -1458,7 +1459,7 @@ self.check_page_for_string( last_hda_created.name ) check_str = 'Manage dataset permissions and role associations of %s' % last_hda_created.name self.check_page_for_string( check_str ) - self.check_page_for_string( 'Role members can manage the roles associated with this dataset' ) + self.check_page_for_string( 'Role members can manage the roles associated with permissions on this dataset' ) self.check_page_for_string( 'Role members can import this dataset into their history for analysis' ) # admin_user is associated with role_one, so should have all permissions on imported datasets check_edit_page1( latest_3_lddas ) @@ -1516,7 +1517,7 @@ pass try: # This should no longer be possible - self.check_page_for_string( 'Role members can manage the roles associated with this dataset' ) + self.check_page_for_string( 'Role members can manage roles associated with permissions on this library item' ) raise AssertionError( '%s incorrectly has DATASET_MANAGE_PERMISSIONS on datasets imported from a library' % admin_user.email ) except: pass