details: http://www.bx.psu.edu/hg/galaxy/rev/d81624fcac0b changeset: 3205:d81624fcac0b user: Greg Von Kuster <greg@bx.psu.edu> date: Tue Jan 05 14:47:55 2010 -0500 description: Bug fixes and code cleanup for managing users, groups and roles. diffstat: lib/galaxy/web/controllers/admin.py | 116 ++++++++------ templates/admin/dataset_security/group/group.mako | 2 +- templates/admin/dataset_security/group/group_rename.mako | 6 +- templates/admin/dataset_security/role/role.mako | 2 +- templates/admin/dataset_security/role/role_rename.mako | 6 +- templates/admin/user/user.mako | 2 +- test/base/twilltestcase.py | 18 +- test/functional/test_security_and_libraries.py | 4 +- 8 files changed, 89 insertions(+), 67 deletions(-) diffs (384 lines): diff -r f87019342f0a -r d81624fcac0b lib/galaxy/web/controllers/admin.py --- a/lib/galaxy/web/controllers/admin.py Tue Jan 05 14:36:09 2010 -0500 +++ b/lib/galaxy/web/controllers/admin.py Tue Jan 05 14:47:55 2010 -0500 @@ -159,7 +159,7 @@ columns = [ NameColumn( "Name", key="name", - link=( lambda item: dict( controller="admin", action="role", id=item.id ) ), + link=( lambda item: dict( operation="Manage users and groups", id=item.id ) ), model_class=model.Role, attach_popup=True, filterable="advanced" ), @@ -187,7 +187,8 @@ global_actions = [ grids.GridAction( "Add new role", dict( controller='admin', action='roles', operation='create' ) ) ] - operations = [ grids.GridOperation( "Delete", condition=( lambda item: not item.deleted ), allow_multiple=True ), + operations = [ grids.GridOperation( "Rename", condition=( lambda item: not item.deleted ), allow_multiple=False ), + grids.GridOperation( "Delete", condition=( lambda item: not item.deleted ), allow_multiple=True ), grids.GridOperation( "Undelete", condition=( lambda item: item.deleted ), allow_multiple=True ), grids.GridOperation( "Purge", condition=( lambda item: item.deleted ), allow_multiple=True ) ] standard_filters = [ @@ -243,7 +244,7 @@ columns = [ NameColumn( "Name", key="name", - link=( lambda item: dict( controller="admin", action="group", id=item.id ) ), + link=( lambda item: dict( operation="Manage users and roles", id=item.id ) ), model_class=model.Group, attach_popup=True, filterable="advanced" ), @@ -261,7 +262,8 @@ global_actions = [ grids.GridAction( "Add new group", dict( controller='admin', action='groups', operation='create' ) ) ] - operations = [ grids.GridOperation( "Delete", condition=( lambda item: not item.deleted ), allow_multiple=True ), + operations = [ grids.GridOperation( "Rename", condition=( lambda item: not item.deleted ), allow_multiple=False ), + grids.GridOperation( "Delete", condition=( lambda item: not item.deleted ), allow_multiple=True ), grids.GridOperation( "Undelete", condition=( lambda item: item.deleted ), allow_multiple=True ), grids.GridOperation( "Purge", condition=( lambda item: item.deleted ), allow_multiple=True ) ] standard_filters = [ @@ -327,8 +329,10 @@ return self.undelete_role( trans, **kwargs ) if operation == "purge": return self.purge_role( trans, **kwargs ) - if operation == "manage users & groups": - return self.role( trans, **kwargs ) + if operation == "manage users and groups": + return self.manage_users_and_groups_for_role( trans, **kwargs ) + if operation == "rename": + return self.rename_role( trans, **kwargs ) # Render the list view return self.role_list_grid( trans, **kwargs ) @web.expose @@ -389,7 +393,32 @@ messagetype=messagetype ) @web.expose @web.require_admin - def role( self, trans, **kwd ): + def rename_role( self, trans, **kwd ): + params = util.Params( kwd ) + msg = util.restore_text( params.get( 'msg', '' ) ) + messagetype = params.get( 'messagetype', 'done' ) + role = get_role( trans, params.id ) + if params.get( 'rename_role_button', False ): + old_name = role.name + new_name = util.restore_text( params.name ) + new_description = util.restore_text( params.description ) + if not new_name: + msg = 'Enter a valid name' + return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype='error' ) + elif trans.sa_session.query( trans.app.model.Role ).filter( trans.app.model.Role.table.c.name==new_name ).first(): + msg = 'A role with that name already exists' + return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype='error' ) + else: + role.name = new_name + role.description = new_description + trans.sa_session.add( role ) + trans.sa_session.flush() + msg = "Role '%s' has been renamed to '%s'" % ( old_name, new_name ) + return trans.response.send_redirect( web.url_for( action='roles', message=util.sanitize_text( msg ), status='done' ) ) + return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype=messagetype ) + @web.expose + @web.require_admin + def manage_users_and_groups_for_role( self, trans, **kwd ): params = util.Params( kwd ) msg = util.restore_text( params.get( 'msg', '' ) ) messagetype = params.get( 'messagetype', 'done' ) @@ -413,26 +442,7 @@ trans.app.security_agent.set_entity_role_associations( roles=[ role ], users=in_users, groups=in_groups ) trans.sa_session.refresh( role ) msg = "Role '%s' has been updated with %d associated users and %d associated groups" % ( role.name, len( in_users ), len( in_groups ) ) - trans.response.send_redirect( web.url_for( action='roles', message=util.sanitize_text( msg ), status=messagetype ) ) - elif params.get( 'rename', False ): - if params.rename == 'submitted': - old_name = role.name - new_name = util.restore_text( params.name ) - new_description = util.restore_text( params.description ) - if not new_name: - msg = 'Enter a valid name' - return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype='error' ) - elif trans.sa_session.query( trans.app.model.Role ).filter( trans.app.model.Role.table.c.name==new_name ).first(): - msg = 'A role with that name already exists' - return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype='error' ) - else: - role.name = new_name - role.description = new_description - trans.sa_session.add( role ) - trans.sa_session.flush() - msg = "Role '%s' has been renamed to '%s'" % ( old_name, new_name ) - return trans.response.send_redirect( web.url_for( action='roles', message=util.sanitize_text( msg ), status='done' ) ) - return trans.fill_template( '/admin/dataset_security/role/role_rename.mako', role=role, msg=msg, messagetype=messagetype ) + trans.response.send_redirect( web.url_for( action='roles', message=util.sanitize_text( msg ), status=messagetype ) ) in_users = [] out_users = [] in_groups = [] @@ -561,13 +571,38 @@ return self.undelete_group( trans, **kwargs ) if operation == "purge": return self.purge_group( trans, **kwargs ) - if operation == "manage users & roles": - return self.group( trans, **kwargs ) + if operation == "manage users and roles": + return self.manage_users_and_roles_for_group( trans, **kwargs ) + if operation == "rename": + return self.rename_group( trans, **kwargs ) # Render the list view return self.group_list_grid( trans, **kwargs ) @web.expose @web.require_admin - def group( self, trans, **kwd ): + def rename_group( self, trans, **kwd ): + params = util.Params( kwd ) + msg = util.restore_text( params.get( 'msg', '' ) ) + messagetype = params.get( 'messagetype', 'done' ) + group = get_group( trans, params.id ) + if params.get( 'rename_group_button', False ): + old_name = group.name + new_name = util.restore_text( params.name ) + if not new_name: + msg = 'Enter a valid name' + return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype='error' ) + elif trans.sa_session.query( trans.app.model.Group ).filter( trans.app.model.Group.table.c.name==new_name ).first(): + msg = 'A group with that name already exists' + return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype='error' ) + else: + group.name = new_name + trans.sa_session.add( group ) + trans.sa_session.flush() + msg = "Group '%s' has been renamed to '%s'" % ( old_name, new_name ) + return trans.response.send_redirect( web.url_for( action='groups', msg=util.sanitize_text( msg ), messagetype='done' ) ) + return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype=messagetype ) + @web.expose + @web.require_admin + def manage_users_and_roles_for_group( self, trans, **kwd ): params = util.Params( kwd ) msg = util.restore_text( params.get( 'msg', '' ) ) messagetype = params.get( 'messagetype', 'done' ) @@ -579,23 +614,6 @@ trans.sa_session.refresh( group ) msg += "Group '%s' has been updated with %d associated roles and %d associated users" % ( group.name, len( in_roles ), len( in_users ) ) trans.response.send_redirect( web.url_for( action='groups', message=util.sanitize_text( msg ), status=messagetype ) ) - if params.get( 'rename', False ): - if params.rename == 'submitted': - old_name = group.name - new_name = util.restore_text( params.name ) - if not new_name: - msg = 'Enter a valid name' - return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype='error' ) - elif trans.sa_session.query( trans.app.model.Group ).filter( trans.app.model.Group.table.c.name==new_name ).first(): - msg = 'A group with that name already exists' - return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype='error' ) - else: - group.name = new_name - trans.sa_session.add( group ) - trans.sa_session.flush() - msg = "Group '%s' has been renamed to '%s'" % ( old_name, new_name ) - return trans.response.send_redirect( web.url_for( action='groups', msg=util.sanitize_text( msg ), messagetype='done' ) ) - return trans.fill_template( '/admin/dataset_security/group/group_rename.mako', group=group, msg=msg, messagetype=messagetype ) in_roles = [] out_roles = [] in_users = [] @@ -943,7 +961,7 @@ if operation == "information": return self.user_info( trans, **kwargs ) if operation == "manage roles and groups": - return self.user( trans, **kwargs ) + return self.manage_roles_and_groups_for_user( trans, **kwargs ) # Render the list view return self.user_list_grid( trans, **kwargs ) @web.expose @@ -976,7 +994,7 @@ return ac_data @web.expose @web.require_admin - def user( self, trans, **kwd ): + def manage_roles_and_groups_for_user( self, trans, **kwd ): user_id = kwd.get( 'id', None ) message = '' status = '' diff -r f87019342f0a -r d81624fcac0b templates/admin/dataset_security/group/group.mako --- a/templates/admin/dataset_security/group/group.mako Tue Jan 05 14:36:09 2010 -0500 +++ b/templates/admin/dataset_security/group/group.mako Tue Jan 05 14:47:55 2010 -0500 @@ -50,7 +50,7 @@ <div class="toolForm"> <div class="toolFormTitle">Group '${group.name}'</div> <div class="toolFormBody"> - <form name="associate_group_role_user" id="associate_group_role_user" action="${h.url_for( action='group', group_id=group.id )}" method="post" > + <form name="associate_group_role_user" id="associate_group_role_user" action="${h.url_for( action='manage_users_and_roles_for_group', id=trans.security.encode_id( group.id ) )}" method="post" > <div class="form-row"> <div style="float: left; margin-right: 10px;"> <label>Roles associated with '${group.name}'</label> diff -r f87019342f0a -r d81624fcac0b templates/admin/dataset_security/group/group_rename.mako --- a/templates/admin/dataset_security/group/group_rename.mako Tue Jan 05 14:36:09 2010 -0500 +++ b/templates/admin/dataset_security/group/group_rename.mako Tue Jan 05 14:47:55 2010 -0500 @@ -8,7 +8,7 @@ <div class="toolForm"> <div class="toolFormTitle">Change group name</div> <div class="toolFormBody"> - <form name="library" action="${h.url_for( controller='admin', action='group' )}" method="post" > + <form name="library" action="${h.url_for( controller='admin', action='rename_group' )}" method="post" > <div class="form-row"> <label>Name:</label> <div style="float: left; width: 250px; margin-right: 10px;"> @@ -28,7 +28,9 @@ </div> <div style="clear: both"></div> </div> - <input type="submit" name="rename_group_button" value="Save"/> + <div class="form-row"> + <input type="submit" name="rename_group_button" value="Save"/> + </div> </form> </div> </div> diff -r f87019342f0a -r d81624fcac0b templates/admin/dataset_security/role/role.mako --- a/templates/admin/dataset_security/role/role.mako Tue Jan 05 14:36:09 2010 -0500 +++ b/templates/admin/dataset_security/role/role.mako Tue Jan 05 14:47:55 2010 -0500 @@ -50,7 +50,7 @@ <div class="toolForm"> <div class="toolFormTitle">Role '${role.name}'</div> <div class="toolFormBody"> - <form name="associate_role_user_group" id="associate_role_user_group" action="${h.url_for( action='role', role_id=role.id )}" method="post" > + <form name="associate_role_user_group" id="associate_role_user_group" action="${h.url_for( action='manage_users_and_groups_for_role', id=trans.security.encode_id( role.id ) )}" method="post" > <div class="form-row"> <div style="float: left; margin-right: 10px;"> <label>Users associated with '${role.name}'</label> diff -r f87019342f0a -r d81624fcac0b templates/admin/dataset_security/role/role_rename.mako --- a/templates/admin/dataset_security/role/role_rename.mako Tue Jan 05 14:36:09 2010 -0500 +++ b/templates/admin/dataset_security/role/role_rename.mako Tue Jan 05 14:47:55 2010 -0500 @@ -8,7 +8,7 @@ <div class="toolForm"> <div class="toolFormTitle">Change role name and description</div> <div class="toolFormBody"> - <form name="library" action="${h.url_for( controller='admin', action='role' )}" method="post" > + <form name="library" action="${h.url_for( controller='admin', action='rename_role' )}" method="post" > <div class="form-row"> <label>Name:</label> <div style="float: left; width: 250px; margin-right: 10px;"> @@ -35,7 +35,9 @@ </div> <div style="clear: both"></div> </div> - <input type="submit" name="rename_role_button" value="Save"/> + <div class="form-row"> + <input type="submit" name="rename_role_button" value="Save"/> + </div> </form> </div> </div> diff -r f87019342f0a -r d81624fcac0b templates/admin/user/user.mako --- a/templates/admin/user/user.mako Tue Jan 05 14:36:09 2010 -0500 +++ b/templates/admin/user/user.mako Tue Jan 05 14:47:55 2010 -0500 @@ -50,7 +50,7 @@ <div class="toolForm"> <div class="toolFormTitle">User '${user.email}'</div> <div class="toolFormBody"> - <form name="associate_user_role_group" id="associate_user_role_group" action="${h.url_for( action='user', id=trans.app.security.encode_id( user.id ) )}" method="post" > + <form name="associate_user_role_group" id="associate_user_role_group" action="${h.url_for( action='manage_roles_and_groups_for_user', id=trans.security.encode_id( user.id ) )}" method="post" > <div class="form-row"> <div style="float: left; margin-right: 10px;"> <label>Roles associated with '${user.email}'</label> diff -r f87019342f0a -r d81624fcac0b test/base/twilltestcase.py --- a/test/base/twilltestcase.py Tue Jan 05 14:36:09 2010 -0500 +++ b/test/base/twilltestcase.py Tue Jan 05 14:47:55 2010 -0500 @@ -960,21 +960,21 @@ def mark_user_deleted( self, user_id, email='' ): """Mark a user as deleted""" self.home() - self.visit_url( "%s/admin/mark_user_deleted?id=%s" % ( self.url, user_id ) ) + self.visit_url( "%s/admin/users?operation=delete&id=%s" % ( self.url, user_id ) ) check_str = "Deleted 1 users" self.check_page_for_string( check_str ) self.home() def undelete_user( self, user_id, email='' ): """Undelete a user""" self.home() - self.visit_url( "%s/admin/undelete_user?id=%s" % ( self.url, user_id ) ) + self.visit_url( "%s/admin/users?operation=undelete&id=%s" % ( self.url, user_id ) ) check_str = "Undeleted 1 users" self.check_page_for_string( check_str ) self.home() def purge_user( self, user_id, email ): """Purge a user account""" self.home() - self.visit_url( "%s/admin/purge_user?id=%s" % ( self.url, user_id ) ) + self.visit_url( "%s/admin/users?operation=purge&id=%s" % ( self.url, user_id ) ) check_str = "Purged 1 users" self.check_page_for_string( check_str ) self.home() @@ -983,7 +983,7 @@ in_group_ids=[], out_group_ids=[], check_str='' ): self.home() - url = "%s/admin/user?id=%s&user_roles_groups_edit_button=Save" % ( self.url, user_id ) + url = "%s/admin/manage_roles_and_groups_for_user?id=%s&user_roles_groups_edit_button=Save" % ( self.url, user_id ) if in_role_ids: url += "&in_roles=%s" % ','.join( in_role_ids ) if out_role_ids: @@ -1006,7 +1006,7 @@ create_group_for_role='no', private_role='' ): """Create a new role""" - url = "%s/admin/create_role?create_role_button=Save&name=%s&description=%s" % ( self.url, name.replace( ' ', '+' ), description.replace( ' ', '+' ) ) + url = "%s/admin/roles?operation=create&create_role_button=Save&name=%s&description=%s" % ( self.url, name.replace( ' ', '+' ), description.replace( ' ', '+' ) ) if in_user_ids: url += "&in_users=%s" % ','.join( in_user_ids ) if in_group_ids: @@ -1038,7 +1038,7 @@ def rename_role( self, role_id, name='Role One Renamed', description='This is Role One Re-described' ): """Rename a role""" self.home() - self.visit_url( "%s/admin/role?rename=True&id=%s" % ( self.url, role_id ) ) + self.visit_url( "%s/admin/roles?operation=rename&id=%s" % ( self.url, role_id ) ) self.check_page_for_string( 'Change role name and description' ) tc.fv( "1", "name", name ) tc.fv( "1", "description", description ) @@ -1081,7 +1081,7 @@ # Tests associated with groups def create_group( self, name='Group One', in_user_ids=[], in_role_ids=[] ): """Create a new group""" - url = "%s/admin/create_group?create_group_button=Save&name=%s" % ( self.url, name.replace( ' ', '+' ) ) + url = "%s/admin/groups?operation=create&create_group_button=Save&name=%s" % ( self.url, name.replace( ' ', '+' ) ) if in_user_ids: url += "&in_users=%s" % ','.join( in_user_ids ) if in_role_ids: @@ -1097,14 +1097,14 @@ def rename_group( self, group_id, name='Group One Renamed' ): """Rename a group""" self.home() - self.visit_url( "%s/admin/group?rename=True&id=%s" % ( self.url, group_id ) ) + self.visit_url( "%s/admin/groups?operation=rename&id=%s" % ( self.url, group_id ) ) self.check_page_for_string( 'Change group name' ) tc.fv( "1", "name", name ) tc.submit( "rename_group_button" ) self.home() def associate_users_and_roles_with_group( self, group_id, group_name, user_ids=[], role_ids=[] ): self.home() - url = "%s/admin/group?id=%s&group_roles_users_edit_button=Save" % ( self.url, group_id ) + url = "%s/admin/manage_users_and_roles_for_group?id=%s&group_roles_users_edit_button=Save" % ( self.url, group_id ) if user_ids: url += "&in_users=%s" % ','.join( user_ids ) if role_ids: diff -r f87019342f0a -r d81624fcac0b test/functional/test_security_and_libraries.py --- a/test/functional/test_security_and_libraries.py Tue Jan 05 14:36:09 2010 -0500 +++ b/test/functional/test_security_and_libraries.py Tue Jan 05 14:47:55 2010 -0500 @@ -21,7 +21,7 @@ self.check_page_for_string( not_logged_in_security_msg ) self.visit_url( "%s/admin/create_role" % self.url ) self.check_page_for_string( not_logged_in_security_msg ) - self.visit_url( "%s/admin/role" % self.url ) + self.visit_url( "%s/admin/manage_users_and_groups_for_role" % self.url ) self.check_page_for_string( not_logged_in_security_msg ) self.visit_url( "%s/admin/groups" % self.url ) self.check_page_for_string( not_logged_in_security_msg ) @@ -76,7 +76,7 @@ raise AssertionError( 'The DefaultHistoryPermission.action for history id %d is "%s", but it should be "%s"' \ % ( latest_history.id, dhp.action, galaxy.model.Dataset.permitted_actions.DATASET_MANAGE_PERMISSIONS.action ) ) self.home() - self.visit_url( "%s/admin/user?id=%s" % ( self.url, self.security.encode_id( admin_user.id ) ) ) + self.visit_url( "%s/admin/manage_roles_and_groups_for_user?id=%s" % ( self.url, self.security.encode_id( admin_user.id ) ) ) self.check_page_for_string( admin_user.email ) # Try deleting the admin_user's private role check_str = "You cannot eliminate a user's private role association."