1 new changeset in galaxy-central: http://bitbucket.org/galaxy/galaxy-central/changeset/bb51baa20151/ changeset: bb51baa20151 user: greg date: 2011-07-29 17:05:55 summary: Fix bug discovered by Glen Beane in rendering roles on the permissions page for data libraries. In the data library hierarchy, rendered roles are all derived from roles associated with the LIBRARY_ACCESS permission, but roles rendered for that permission itself must be handled as a special case, which now is the case in that all legitimate roles not associated are listed. affected #: 4 files (701 bytes) --- a/lib/galaxy/security/__init__.py Thu Jul 28 11:50:56 2011 -0400 +++ b/lib/galaxy/security/__init__.py Fri Jul 29 11:05:55 2011 -0400 @@ -90,6 +90,8 @@ raise "Unimplemented Method" def get_permissions( self, library_dataset ): raise "Unimplemented Method" + def get_all_roles( self, trans, cntrller ): + raise "Unimplemented Method" def get_legitimate_roles( self, trans, item, cntrller ): raise "Unimplemented Method" def derive_roles_from_access( self, trans, item_id, cntrller, library=False, **kwd ): @@ -120,6 +122,50 @@ def sa_session( self ): """Returns a SQLAlchemy session""" return self.model.context + def sort_by_attr( self, seq, attr ): + """ + Sort the sequence of objects by object's attribute + Arguments: + seq - the list or any sequence (including immutable one) of objects to sort. + attr - the name of attribute to sort by + """ + # Use the "Schwartzian transform" + # Create the auxiliary list of tuples where every i-th tuple has form + # (seq[i].attr, i, seq[i]) and sort it. The second item of tuple is needed not + # only to provide stable sorting, but mainly to eliminate comparison of objects + # (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 ) ) + def get_all_roles( self, trans, cntrller ): + admin_controller = cntrller in [ 'library_admin' ] + roles = set() + if not trans.user: + return trans.sa_session.query( trans.app.model.Role ) \ + .filter( and_( self.model.Role.table.c.deleted==False, + self.model.Role.table.c.type != self.model.Role.types.PRIVATE, + self.model.Role.table.c.type != self.model.Role.types.SHARING ) ) \ + .order_by( self.model.Role.table.c.name ) + if admin_controller: + # The library is public and the user is an admin, so all roles are legitimate + for role in trans.sa_session.query( trans.app.model.Role ) \ + .filter( self.model.Role.table.c.deleted==False ) \ + .order_by( self.model.Role.table.c.name ): + roles.add( role ) + else: + # Add the current user's private role + roles.add( self.get_private_user_role( trans.user ) ) + # Add the current user's sharing roles + for role in self.get_sharing_roles( trans.user ): + roles.add( role ) + # Add all remaining non-private, non-sharing roles + for role in trans.sa_session.query( trans.app.model.Role ) \ + .filter( and_( self.model.Role.table.c.deleted==False, + self.model.Role.table.c.type != self.model.Role.types.PRIVATE, + self.model.Role.table.c.type != self.model.Role.types.SHARING ) ) \ + .order_by( self.model.Role.table.c.name ): + roles.add( role ) + return self.sort_by_attr( [ role for role in roles ], 'name' ) def get_legitimate_roles( self, trans, item, cntrller ): """ Return a sorted list of legitimate roles that can be associated with a permission on @@ -140,51 +186,10 @@ for the current user's private role, will be excluded. """ admin_controller = cntrller in [ 'library_admin' ] - def sort_by_attr( seq, attr ): - """ - Sort the sequence of objects by object's attribute - Arguments: - seq - the list or any sequence (including immutable one) of objects to sort. - attr - the name of attribute to sort by - """ - # Use the "Schwartzian transform" - # Create the auxiliary list of tuples where every i-th tuple has form - # (seq[i].attr, i, seq[i]) and sort it. The second item of tuple is needed not - # only to provide stable sorting, but mainly to eliminate comparison of objects - # (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 ) ) roles = set() 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 not trans.user: - return trans.sa_session.query( trans.app.model.Role ) \ - .filter( and_( self.model.Role.table.c.deleted==False, - self.model.Role.table.c.type != self.model.Role.types.PRIVATE, - self.model.Role.table.c.type != self.model.Role.types.SHARING ) ) \ - .order_by( self.model.Role.table.c.name ) - if admin_controller: - # The library is public and the user is an admin, so all roles are legitimate - for role in trans.sa_session.query( trans.app.model.Role ) \ - .filter( self.model.Role.table.c.deleted==False ) \ - .order_by( self.model.Role.table.c.name ): - roles.add( role ) - return sort_by_attr( [ role for role in roles ], 'name' ) - else: - # Add the current user's private role - roles.add( self.get_private_user_role( trans.user ) ) - # Add the current user's sharing roles - for role in self.get_sharing_roles( trans.user ): - roles.add( role ) - # Add all remaining non-private, non-sharing roles - for role in trans.sa_session.query( trans.app.model.Role ) \ - .filter( and_( self.model.Role.table.c.deleted==False, - self.model.Role.table.c.type != self.model.Role.types.PRIVATE, - self.model.Role.table.c.type != self.model.Role.types.SHARING ) ) \ - .order_by( self.model.Role.table.c.name ): - roles.add( role ) - return sort_by_attr( [ role for role in roles ], 'name' ) + return self.get_all_roles( trans, cntrller ) # If item has roles associated with the access permission, we need to start with them. access_roles = item.get_access_roles( trans ) for role in access_roles: @@ -205,7 +210,7 @@ for ura in user.roles: if admin_controller or self.ok_to_display( trans.user, ura.role ): roles.add( ura.role ) - return sort_by_attr( [ role for role in roles ], 'name' ) + return self.sort_by_attr( [ role for role in roles ], 'name' ) def ok_to_display( self, user, role ): """ Method for checking if: --- a/lib/galaxy/web/controllers/library_common.py Thu Jul 28 11:50:56 2011 -0400 +++ b/lib/galaxy/web/controllers/library_common.py Fri Jul 29 11:05:55 2011 -0400 @@ -246,12 +246,14 @@ message=util.sanitize_text( message ), status='done' ) ) roles = trans.app.security_agent.get_legitimate_roles( trans, library, cntrller ) + all_roles = trans.app.security_agent.get_all_roles( trans, cntrller ) return trans.fill_template( '/library/common/library_permissions.mako', cntrller=cntrller, use_panels=use_panels, library=library, current_user_roles=current_user_roles, roles=roles, + all_roles=all_roles, show_deleted=show_deleted, message=message, status=status ) --- a/templates/dataset/security_common.mako Thu Jul 28 11:50:56 2011 -0400 +++ b/templates/dataset/security_common.mako Fri Jul 29 11:05:55 2011 -0400 @@ -1,11 +1,11 @@ -<%def name="render_select( current_actions, action_key, action, all_roles )"> +<%def name="render_select( current_actions, action_key, action, roles )"><% import sets in_roles = sets.Set() for a in current_actions: if a.action == action.action: in_roles.add( a.role ) - out_roles = filter( lambda x: x not in in_roles, all_roles ) + out_roles = filter( lambda x: x not in in_roles, roles ) %><p><b>${action.action}:</b> ${action.description} @@ -37,7 +37,7 @@ </%def> ## Any permission ( e.g., 'DATASET_ACCESS' ) included in the do_not_render param will not be rendered on the page. -<%def name="render_permission_form( obj, obj_name, form_url, all_roles, do_not_render=[] )"> +<%def name="render_permission_form( obj, obj_name, form_url, roles, do_not_render=[], all_roles=[] )"><% if isinstance( obj, trans.app.model.User ): current_actions = obj.default_permissions @@ -109,7 +109,14 @@ %for k, v in permitted_actions: %if k not in do_not_render: <div class="form-row"> - ${render_select( current_actions, k, v, all_roles )} + # LIBRARY_ACCESS is a special case because we need to render all roles instead of + # roles derived from the roles associated with LIBRARY_ACCESS. + <% render_all_roles = k == 'LIBRARY_ACCESS' %> + %if render_all_roles: + ${render_select( current_actions, k, v, all_roles )} + %else: + ${render_select( current_actions, k, v, roles )} + %endif </div> %endif %endfor --- a/templates/library/common/library_permissions.mako Thu Jul 28 11:50:56 2011 -0400 +++ b/templates/library/common/library_permissions.mako Fri Jul 29 11:05:55 2011 -0400 @@ -14,5 +14,5 @@ %endif %if ( trans.user_is_admin() and cntrller == 'library_admin' ) or trans.app.security_agent.can_manage_library_item( current_user_roles, library ): - ${render_permission_form( library, library.name, h.url_for( controller='library_common', action='library_permissions', cntrller=cntrller, id=trans.security.encode_id( library.id ), show_deleted=show_deleted ), roles )} + ${render_permission_form( library, library.name, h.url_for( controller='library_common', action='library_permissions', cntrller=cntrller, id=trans.security.encode_id( library.id ), show_deleted=show_deleted ), roles, do_not_render=[], all_roles=all_roles )} %endif Repository URL: https://bitbucket.org/galaxy/galaxy-central/ -- This is a commit notification from bitbucket.org. You are receiving this because you have the service enabled, addressing the recipient of this email.