details: http://www.bx.psu.edu/hg/galaxy/rev/fef431822a73 changeset: 3293:fef431822a73 user: Greg Von Kuster greg@bx.psu.edu date: Thu Jan 28 09:05:13 2010 -0500 description: Derive roles for the permissions forms on the edit attributes page from the roles associated with the access permission on the dataset. Addd error checking to ensure the user does not set access permissions in such a way that the dataset is inaccessible to himself or everyone. Fix a bug when clicking on a library dataset to view it's info from the Data Libraries view.

diffstat:

lib/galaxy/security/__init__.py | 40 +++++++++++++++++--------- lib/galaxy/web/controllers/library_common.py | 7 ++-- lib/galaxy/web/controllers/root.py | 41 ++++++++++++++++++++++----- lib/galaxy/web/controllers/tool_runner.py | 2 +- templates/dataset/edit_attributes.mako | 4 ++ 5 files changed, 68 insertions(+), 26 deletions(-)

diffs (221 lines):

diff -r 19334cf7f9b4 -r fef431822a73 lib/galaxy/security/__init__.py --- a/lib/galaxy/security/__init__.py Wed Jan 27 18:34:36 2010 -0500 +++ b/lib/galaxy/security/__init__.py Thu Jan 28 09:05:13 2010 -0500 @@ -81,7 +81,7 @@ raise "Unimplemented Method" def get_legitimate_roles( self, trans, item ): raise "Unimplemented Method" - def check_library_dataset_access( self, trans, library_id, **kwd ): + def derive_roles_from_access( self, trans, item_id, library=False, **kwd ): raise "Unimplemented Method" def get_component_associations( self, **kwd ): raise "Unimplemented Method" @@ -119,6 +119,7 @@ """ if ( isinstance( item, self.model.Library ) and self.library_is_public( item ) ) or \ ( isinstance( item, self.model.Dataset ) and self.dataset_is_public( item ) ): + # If item is public, private roles will always be allowed return trans.sa_session.query( trans.app.model.Role ) \ .filter( trans.app.model.Role.table.c.deleted==False ) \ .order_by( trans.app.model.Role.table.c.name ) @@ -136,7 +137,7 @@ # (which can be expensive or prohibited) in case of equal attribute values. intermed = map( None, map( getattr, seq, ( attr, ) * len( seq ) ), xrange( len( seq ) ), seq ) intermed.sort() - return map( operator.getitem, intermed, ( -1, ) * len( intermed ) ) + return map( operator.getitem, intermed, ( -1, ) * len( intermed ) ) roles = set() # If item has roles associated with the access permission, we need to start with them. access_roles = item.get_access_roles( trans ) @@ -172,7 +173,7 @@ break return ret_val def can_access_dataset( self, roles, dataset ): - return self.allow_action( roles, self.permitted_actions.DATASET_ACCESS, dataset ) + return self.dataset_is_public( dataset ) or self.allow_action( roles, self.permitted_actions.DATASET_ACCESS, dataset ) def can_manage_dataset( self, roles, dataset ): return self.allow_action( roles, self.permitted_actions.DATASET_MANAGE_PERMISSIONS, dataset ) def can_access_library( self, roles, library ): @@ -452,8 +453,9 @@ if lp.action == self.permitted_actions.LIBRARY_ACCESS.action: self.sa_session.delete( lp ) self.sa_session.flush() - def check_library_dataset_access( self, trans, library_id, **kwd ): - # library_id must be decoded before being sent + def derive_roles_from_access( self, trans, item_id, library=False, **kwd ): + # Check the access permission on a dataset. If library is true, item_id refers to a library. If library + # is False, item_id refers to a dataset ( item_id must currently be decoded before being sent ). msg = '' permissions = {} # accessible will be True only if at least 1 user has every role in DATASET_ACCESS_in @@ -465,22 +467,32 @@ for k, v in get_permitted_actions( filter='DATASET' ).items(): in_roles = [ self.sa_session.query( self.model.Role ).get( x ) for x in listify( kwd.get( k + '_in', [] ) ) ] if v == self.permitted_actions.DATASET_ACCESS and in_roles: - library = self.model.Library.get( library_id ) - if not self.library_is_public( library ): + if library: + item = self.model.Library.get( item_id ) + else: + item = self.model.Dataset.get( item_id ) + if ( library and not self.library_is_public( item ) ) or ( not library and not self.dataset_is_public( item ) ): # Ensure that roles being associated with DATASET_ACCESS are a subset of the legitimate roles - # derived from the roles associated with the LIBRARY_ACCESS permission on the library if it's - # not public. This will keep ill-legitimate roles from being associated with the DATASET_ACCESS - # permission on the dataset (i.e., if Role1 is associated with LIBRARY_ACCESS, then only those users - # that have Role1 should be associated with DATASET_ACCESS. - legitimate_roles = self.get_legitimate_roles( trans, library ) + # derived from the roles associated with the access permission on item if it's not public. This + # will keep ill-legitimate roles from being associated with the DATASET_ACCESS permission on the + # dataset (i.e., in the case where item is a library, if Role1 is associated with LIBRARY_ACCESS, + # then only those users that have Role1 should be associated with DATASET_ACCESS. + legitimate_roles = self.get_legitimate_roles( trans, item ) ill_legitimate_roles = [] for role in in_roles: if role not in legitimate_roles: ill_legitimate_roles.append( role ) if ill_legitimate_roles: + # NOTE: this condition should never occur since ill-legitimate roles are filtered out of the set of + # roles displayed on the permissions forms, but just in case there is a bug somewhere that incorrectly + # filters, we'll display this message. error = self.ILL_LEGITIMATE - msg += "The following roles are not associated with users that have the 'access library' permission on this " - msg += "library, so they cannot be associated with the 'access' permission on the datasets: " + if library: + msg += "The following roles are not associated with users that have the 'access library' permission on this " + msg += "library, so they cannot be associated with the 'access' permission on the datasets: " + else: + msg += "The following roles are not associated with users that have the 'access' permission on this " + msg += "dataset, so they were incorrectly displayed on the permission form: " for role in ill_legitimate_roles: msg += "%s, " % role.name msg = msg.rstrip( ", " ) diff -r 19334cf7f9b4 -r fef431822a73 lib/galaxy/web/controllers/library_common.py --- a/lib/galaxy/web/controllers/library_common.py Wed Jan 27 18:34:36 2010 -0500 +++ b/lib/galaxy/web/controllers/library_common.py Thu Jan 28 09:05:13 2010 -0500 @@ -469,6 +469,7 @@ library=library, show_deleted=show_deleted, widgets=widgets, + current_user_roles=current_user_roles, msg=msg, messagetype=messagetype ) @web.expose @@ -506,11 +507,11 @@ if cntrller=='library_admin' or ( trans.app.security_agent.can_manage_library_item( current_user_roles, ldda ) and \ trans.app.security_agent.can_manage_dataset( current_user_roles, ldda.dataset ) ): permissions, in_roles, error, msg = \ - trans.app.security_agent.check_library_dataset_access( trans, trans.app.security.decode_id( library_id ), **kwd ) + trans.app.security_agent.derive_roles_from_access( trans, trans.app.security.decode_id( library_id ), library=True, **kwd ) for ldda in lddas: # Set the DATASET permissions on the Dataset. if error == trans.app.security_agent.IN_ACCESSIBLE: - # If the check_library_dataset_access() returned a "in_accessible" error, then we keep the original role + # If derive_roles_from_access() returned an "in_accessible" error, then we keep the original role # associations for the DATASET_ACCESS permission on each ldda. a = trans.app.security_agent.get_action( trans.app.security_agent.permitted_actions.DATASET_ACCESS.action ) permissions[ a ] = ldda.get_access_roles( trans ) @@ -619,7 +620,7 @@ if roles: vars = dict( DATASET_ACCESS_in=roles ) permissions, in_roles, error, msg = \ - trans.app.security_agent.check_library_dataset_access( trans, trans.app.security.decode_id( library_id ), **vars ) + trans.app.security_agent.derive_roles_from_access( trans, trans.app.security.decode_id( library_id ), library=True, **vars ) if error: if error == trans.app.security_agent.IN_ACCESSIBLE: msg = "At least 1 user must have every role associated with accessing datasets. The roles you " diff -r 19334cf7f9b4 -r fef431822a73 lib/galaxy/web/controllers/root.py --- a/lib/galaxy/web/controllers/root.py Wed Jan 27 18:34:36 2010 -0500 +++ b/lib/galaxy/web/controllers/root.py Thu Jan 28 09:05:13 2010 -0500 @@ -240,6 +240,8 @@ @web.expose def edit(self, trans, id=None, hid=None, **kwd): """Allows user to modify parameters of an HDA.""" + msg = '' + error = False def __ok_to_edit_metadata( dataset_id ): #prevent modifying metadata when dataset is queued or running as input/output #This code could be more efficient, i.e. by using mappers, but to prevent slowing down loading a History panel, we'll leave the code here for now @@ -334,13 +336,29 @@ if not trans.user: return trans.show_error_message( "You must be logged in if you want to change permissions." ) if trans.app.security_agent.can_manage_dataset( current_user_roles, data.dataset ): - permissions = {} - for k, v in trans.app.model.Dataset.permitted_actions.items(): - in_roles = params.get( k + '_in', [] ) - if not isinstance( in_roles, list ): - in_roles = [ in_roles ] - in_roles = [ trans.sa_session.query( trans.app.model.Role ).get( x ) for x in in_roles ] - permissions[ trans.app.security_agent.get_action( v.action ) ] = in_roles + # The user associated the DATASET_ACCESS permission on the dataset with 1 or more roles. We need + # to ensure that they did not associated roles that would make the dataset in-accessible by everyone. + permissions, in_roles, error, msg = \ + trans.app.security_agent.derive_roles_from_access( trans, data.dataset.id, **kwd ) + a = trans.app.security_agent.get_action( trans.app.security_agent.permitted_actions.DATASET_ACCESS.action ) + if error == trans.app.security_agent.IN_ACCESSIBLE: + # If derive_roles_from_access() returned an "in_accessible" error, then we keep the original role + # associations for the DATASET_ACCESS permission on the dataset. + permissions[ a ] = data.dataset.get_access_roles( trans ) + # Make sure the user is not associating another user's private role with the DATASET_ACCESS permission. + # It would be better to filter out other user's private roles from the access box on the permission form, + # but that gets a bit tricky since we are not differentiating between permissions ( i.e., the same set of + # derived roles are used for both the manage permissions and the access box ). + current_user_private_role = trans.app.security_agent.get_private_user_role( trans.user ) + access_roles = permissions[ a ] + for role in access_roles: + if role.type == trans.app.model.Role.types.PRIVATE and role is not current_user_private_role: + error = trans.app.security_agent.IN_ACCESSIBLE + permissions[ a ] = data.dataset.get_access_roles( trans ) + msg = "You cannot associate another user's private role with the access permission on this dataset " + msg += "or it will become in-accessible to you. Access permissions were left in their original state." + messagetype = 'error' + break trans.app.security_agent.set_all_dataset_permissions( data.dataset, permissions ) trans.sa_session.refresh( data.dataset ) else: @@ -358,11 +376,18 @@ ldatatypes = [ dtype_name for dtype_name, dtype_value in trans.app.datatypes_registry.datatypes_by_extension.iteritems() if dtype_value.allow_datatype_change ] ldatatypes.sort() all_roles = trans.app.security_agent.get_legitimate_roles( trans, data.dataset ) + if error: + messagetype = 'error' + else: + msg = 'Your changes completed successfully.' + messagetype = 'done' return trans.fill_template( "/dataset/edit_attributes.mako", data=data, datatypes=ldatatypes, current_user_roles=current_user_roles, - all_roles=all_roles ) + all_roles=all_roles, + msg=msg, + messagetype=messagetype ) else: return trans.show_error_message( "You do not have permission to edit this dataset's ( id: %s ) information." % str( id ) )

diff -r 19334cf7f9b4 -r fef431822a73 lib/galaxy/web/controllers/tool_runner.py --- a/lib/galaxy/web/controllers/tool_runner.py Wed Jan 27 18:34:36 2010 -0500 +++ b/lib/galaxy/web/controllers/tool_runner.py Thu Jan 28 09:05:13 2010 -0500 @@ -151,7 +151,7 @@ # did not associated roles that would make the dataset in-accessible by everyone. library_id = trans.app.security.decode_id( kwd.get( 'library_id', '' ) ) vars = dict( DATASET_ACCESS_in=roles ) - permissions, in_roles, error, msg = trans.app.security_agent.check_library_dataset_access( trans, library_id, **vars ) + permissions, in_roles, error, msg = trans.app.security_agent.check_access_permission( trans, library_id, library=True, **vars ) if error: return [ 'error', msg ] permissions = trans.app.security_agent.history_get_default_permissions( trans.history ) diff -r 19334cf7f9b4 -r fef431822a73 templates/dataset/edit_attributes.mako --- a/templates/dataset/edit_attributes.mako Wed Jan 27 18:34:36 2010 -0500 +++ b/templates/dataset/edit_attributes.mako Thu Jan 28 09:05:13 2010 -0500 @@ -30,6 +30,10 @@ </select> </%def>

+%if msg: + ${render_msg( msg, messagetype )} +%endif + <div class="toolForm"> <div class="toolFormTitle">${_('Edit Attributes')}</div> <div class="toolFormBody">