details: http://www.bx.psu.edu/hg/galaxy/rev/a1e84abd301a changeset: 2367:a1e84abd301a user: Greg Von Kuster <greg@bx.psu.edu> date: Fri Apr 24 13:42:38 2009 -0400 description: Two fixes: add type checking for id when deleting datasets, ad checks to make sure at least 1 user can access a set of datasets when setting permissions on multiple datasets. 3 file(s) affected in this change: lib/galaxy/web/controllers/admin.py lib/galaxy/web/controllers/dataset.py test/functional/test_security_and_libraries.py diffs (114 lines): diff -r e7492f6c18ca -r a1e84abd301a lib/galaxy/web/controllers/admin.py --- a/lib/galaxy/web/controllers/admin.py Fri Apr 24 13:03:57 2009 -0400 +++ b/lib/galaxy/web/controllers/admin.py Fri Apr 24 13:42:38 2009 -0400 @@ -1495,9 +1495,36 @@ if action == 'permissions': if params.get( 'update_roles_button', False ): permissions = {} + accessible = False for k, v in trans.app.model.Dataset.permitted_actions.items(): in_roles = [ trans.app.model.Role.get( x ) for x in util.listify( params.get( k + '_in', [] ) ) ] - permissions[ trans.app.security_agent.get_action( v.action ) ] = in_roles + # At least 1 user must have every role associated with this dataset, or the dataset is inaccessible + if v == trans.app.security_agent.permitted_actions.DATASET_ACCESS: + if len( in_roles ) > 1: + # Get the set of all users that are being associated with the dataset + in_roles_set = sets.Set() + for role in in_roles: + in_roles_set.add( role ) + users_set = sets.Set() + for role in in_roles: + for ura in role.users: + users_set.add( ura.user ) + # Make sure that at least 1 user has every role being associated with the dataset + for user in users_set: + user_roles_set = sets.Set() + for ura in user.roles: + user_roles_set.add( ura.role ) + if in_roles_set.issubset( user_roles_set ): + accessible = True + break + else: + accessible = True + if not accessible and v == trans.app.security_agent.permitted_actions.DATASET_ACCESS: + # Don't set the permissions for DATASET_ACCESS if inaccessbile, but set all other permissions + # TODO: keep access permissions as they originally were, rather than automatically making public + permissions[ trans.app.security_agent.get_action( v.action ) ] = [] + else: + permissions[ trans.app.security_agent.get_action( v.action ) ] = in_roles for ldda in lddas: # Set the DATASET permissions on the Dataset trans.app.security_agent.set_all_dataset_permissions( ldda.dataset, permissions ) @@ -1514,7 +1541,13 @@ # Set the LIBRARY permissions on the LibraryDatasetDatasetAssociation trans.app.security_agent.set_all_library_permissions( ldda, permissions ) ldda.refresh() - msg = 'Permissions and roles have been updated on %d datasets' % len( lddas ) + if not accessible: + msg = "At least 1 user must have every role associated with accessing these %d datasets. " % len( lddas ) + msg += "The roles you attempted to associate for access would make these datasets inaccessible by everyone, " + msg += "so access permissions were not set. All other permissions were updated for the datasets." + messagetype = 'error' + else: + msg = "Permissions have been updated on %d datasets" % len( lddas ) return trans.fill_template( "/admin/library/ldda_permissions.mako", ldda=lddas, library_id=library_id, diff -r e7492f6c18ca -r a1e84abd301a lib/galaxy/web/controllers/dataset.py --- a/lib/galaxy/web/controllers/dataset.py Fri Apr 24 13:03:57 2009 -0400 +++ b/lib/galaxy/web/controllers/dataset.py Fri Apr 24 13:42:38 2009 -0400 @@ -125,19 +125,20 @@ return trans.show_error_message( "You are not allowed to access this dataset" ) def _undelete( self, trans, id ): - history = trans.get_history() - data = self.app.model.HistoryDatasetAssociation.get( id ) - if data and data.undeletable: - # Walk up parent datasets to find the containing history - topmost_parent = data - while topmost_parent.parent: - topmost_parent = topmost_parent.parent - assert topmost_parent in history.datasets, "Data does not belong to current history" - # Mark undeleted - data.mark_undeleted() - self.app.model.flush() - trans.log_event( "Dataset id %s has been undeleted" % str(id) ) - return True + if isinstance( id, type( 1 ) ): + history = trans.get_history() + data = self.app.model.HistoryDatasetAssociation.get( id ) + if data and data.undeletable: + # Walk up parent datasets to find the containing history + topmost_parent = data + while topmost_parent.parent: + topmost_parent = topmost_parent.parent + assert topmost_parent in history.datasets, "Data does not belong to current history" + # Mark undeleted + data.mark_undeleted() + self.app.model.flush() + trans.log_event( "Dataset id %s has been undeleted" % str(id) ) + return True return False @web.expose diff -r e7492f6c18ca -r a1e84abd301a test/functional/test_security_and_libraries.py --- a/test/functional/test_security_and_libraries.py Fri Apr 24 13:03:57 2009 -0400 +++ b/test/functional/test_security_and_libraries.py Fri Apr 24 13:42:38 2009 -0400 @@ -737,7 +737,7 @@ url = build_url( permissions, role_one ) self.home() self.visit_url( url ) - self.check_page_for_string( 'Permissions and roles have been updated on 3 datasets' ) + self.check_page_for_string( 'Permissions have been updated on 3 datasets' ) def check_edit_page1( lddas ): # Make sure the permissions have been correctly updated for the 3 datasets. Permissions should # be all of the above on any of the 3 datasets that are imported into a history @@ -785,7 +785,7 @@ url = build_url( permissions, role_one ) self.home() self.visit_url( url ) - self.check_page_for_string( 'Permissions and roles have been updated on 3 datasets' ) + self.check_page_for_string( 'Permissions have been updated on 3 datasets' ) def check_edit_page2( lddas ): # Make sure the permissions have been correctly updated for the 3 datasets. Permissions should # be all of the above on any of the 3 datasets that are imported into a history