4 new commits in galaxy-central: https://bitbucket.org/galaxy/galaxy-central/commits/b0d60614d255/ Changeset: b0d60614d255 Branch: stable User: natefoo Date: 2014-12-02 15:42:44+00:00 Summary: Prevent XSS in various user-related templates (OpenID, password reset, manage user, user addresses, admin management of user API keys). Also fix places where the redirect URL used by OpenID methods could point to a site external to Galaxy. Affected #: 7 files diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 lib/galaxy/webapps/galaxy/controllers/user.py --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -28,7 +28,7 @@ from galaxy.web.base.controller import CreatesApiKeysMixin from galaxy.web.form_builder import CheckboxField from galaxy.web.form_builder import build_select_field -from galaxy.web.framework.helpers import time_ago, grids +from galaxy.web.framework.helpers import time_ago, grids, escape from datetime import datetime, timedelta from galaxy.util import hash_util, biostar @@ -164,7 +164,7 @@ user_openid.provider = openid_provider if trans.user: if user_openid.user and user_openid.user.id != trans.user.id: - message = "The OpenID <strong>%s</strong> is already associated with another Galaxy account, <strong>%s</strong>. Please disassociate it from that account before attempting to associate it with a new account." % ( display_identifier, user_openid.user.email ) + message = escape( "The OpenID <strong>%s</strong> is already associated with another Galaxy account, <strong>%s</strong>. Please disassociate it from that account before attempting to associate it with a new account." % ( display_identifier, user_openid.user.email ) ) if not trans.user.active and trans.app.config.user_activation_on: # Account activation is ON and the user is INACTIVE. if ( trans.app.config.activation_grace_period != 0 ): # grace period is ON if self.is_outside_grace_period( trans, trans.user.create_time ): # User is outside the grace period. Login is disabled and he will have the activation email resent. @@ -179,23 +179,23 @@ user_openid.session = trans.galaxy_session if not openid_provider_obj.never_associate_with_user: if not auto_associate and ( user_openid.user and user_openid.user.id == trans.user.id ): - message = "The OpenID <strong>%s</strong> is already associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) + message = escape( "The OpenID <strong>%s</strong> is already associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) ) status = "warning" else: - message = "The OpenID <strong>%s</strong> has been associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) + message = escape( "The OpenID <strong>%s</strong> has been associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) ) status = "done" user_openid.user = trans.user trans.sa_session.add( user_openid ) trans.sa_session.flush() trans.log_event( "User associated OpenID: %s" % display_identifier ) else: - message = "The OpenID <strong>%s</strong> cannot be used to log into your Galaxy account, but any post authentication actions have been performed." % ( openid_provider_obj.name ) + message = escape( "The OpenID <strong>%s</strong> cannot be used to log into your Galaxy account, but any post authentication actions have been performed." % ( openid_provider_obj.name ) ) status = "info" openid_provider_obj.post_authentication( trans, trans.app.openid_manager, info ) if redirect: - message = '%s<br>Click <a href="%s"><strong>here</strong></a> to return to the page you were previously viewing.' % ( message, redirect ) + message = '%s<br>Click <a href="%s"><strong>here</strong></a> to return to the page you were previously viewing.' % ( message, escape( self.__get_redirect_url( redirect ) ) ) if redirect and status != "error": - return trans.response.send_redirect( redirect ) + return trans.response.send_redirect( self.__get_redirect_url( redirect ) ) return trans.response.send_redirect( url_for( controller='user', action='openid_manage', use_panels=True, @@ -208,6 +208,7 @@ openid_provider_obj.post_authentication( trans, trans.app.openid_manager, info ) if not redirect: redirect = url_for( '/' ) + redirect = self.__get_redirect_url( redirect ) return trans.response.send_redirect( redirect ) trans.sa_session.add( user_openid ) trans.sa_session.flush() @@ -449,13 +450,7 @@ @web.expose def login( self, trans, refresh_frames=[], **kwd ): '''Handle Galaxy Log in''' - redirect = kwd.get( 'redirect', trans.request.referer ).strip() - root_url = url_for( '/', qualified=True ) - redirect_url = '' # always start with redirect_url being empty - # compare urls, to prevent a redirect from pointing (directly) outside of galaxy - # or to enter a logout/login loop - if not util.compare_urls( root_url, redirect, compare_path=False ) or util.compare_urls( url_for( controller='user', action='logout', qualified=True ), redirect ): - redirect = root_url + redirect = self.__get_redirect_url( kwd.get( 'redirect', trans.request.referer ).strip() ) use_panels = util.string_as_bool( kwd.get( 'use_panels', False ) ) message = kwd.get( 'message', '' ) status = kwd.get( 'status', 'done' ) @@ -908,7 +903,7 @@ username = util.restore_text( params.get( 'username', '' ) ) if not username: username = user.username - message = util.restore_text( params.get( 'message', '' ) ) + message = escape( util.restore_text( params.get( 'message', '' ) ) ) status = params.get( 'status', 'done' ) if trans.webapp.name == 'galaxy': user_type_form_definition = self.__get_user_type_form_definition( trans, user=user, **kwd ) @@ -1096,7 +1091,7 @@ if trans.app.config.smtp_server is None: return trans.show_error_message( "Mail is not configured for this Galaxy instance. Please contact your local Galaxy administrator." ) message = util.sanitize_text(util.restore_text( kwd.get( 'message', '' ) )) - status = 'done' + status = kwd.get( 'status', 'done' ) if kwd.get( 'reset_password_button', False ): reset_user = trans.sa_session.query( trans.app.model.User ).filter( trans.app.model.User.table.c.email == email ).first() user = trans.get_user() @@ -1123,7 +1118,7 @@ trans.sa_session.add( reset_user ) trans.sa_session.flush() trans.log_event( "User reset password: %s" % email ) - message = "Password has been reset and emailed to: %s. <a href='%s'>Click here</a> to return to the login form." % ( email, web.url_for( controller='user', action='login' ) ) + message = "Password has been reset and emailed to: %s. <a href='%s'>Click here</a> to return to the login form." % ( escape( email ), web.url_for( controller='user', action='login' ) ) except Exception, e: message = 'Failed to reset password: %s' % str( e ) status = 'error' @@ -1439,7 +1434,7 @@ @web.expose def edit_address( self, trans, cntrller, **kwd ): params = util.Params( kwd ) - message = util.restore_text( params.get( 'message', '' ) ) + message = escape( util.restore_text( params.get( 'message', '' ) ) ) status = params.get( 'status', 'done' ) is_admin = cntrller == 'admin' and trans.user_is_admin() user_id = params.get( 'user_id', False ) @@ -1709,7 +1704,7 @@ @web.require_login() def api_keys( self, trans, cntrller, **kwd ): params = util.Params( kwd ) - message = util.restore_text( params.get( 'message', '' ) ) + message = escape( util.restore_text( params.get( 'message', '' ) ) ) status = params.get( 'status', 'done' ) if params.get( 'new_api_key_button', False ): self.create_api_key( trans, trans.user ) @@ -1721,6 +1716,18 @@ message=message, status=status ) + def __get_redirect_url( self, redirect ): + root_url = url_for( '/', qualified=True ) + redirect_url = '' # always start with redirect_url being empty + # compare urls, to prevent a redirect from pointing (directly) outside of galaxy + # or to enter a logout/login loop + if not util.compare_urls( root_url, redirect, compare_path=False ) or util.compare_urls( url_for( controller='user', action='logout', qualified=True ), redirect ): + log.warning('Redirect URL is outside of Galaxy, will redirect to Galaxy root instead: %s', redirect) + redirect = root_url + elif util.compare_urls( url_for( controller='user', action='logout', qualified=True ), redirect ): + redirect = root_url + return redirect + # ===== Methods for building SelectFields ================================ def __build_user_type_fd_id_select_field( self, trans, selected_value ): # Get all the user information forms diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 lib/galaxy/webapps/galaxy/controllers/userskeys.py --- a/lib/galaxy/webapps/galaxy/controllers/userskeys.py +++ b/lib/galaxy/webapps/galaxy/controllers/userskeys.py @@ -3,12 +3,11 @@ """ import logging -import pprint from galaxy import web from galaxy import util, model from galaxy.web.base.controller import BaseUIController, UsesFormDefinitionsMixin -from galaxy.web.framework.helpers import time_ago, grids +from galaxy.web.framework.helpers import time_ago, grids, escape from inspect import getmembers @@ -21,65 +20,46 @@ <p/> """ -class UserOpenIDGrid( grids.Grid ): - use_panels = False - title = "OpenIDs linked to your account" - model_class = model.UserOpenID - template = '/user/openid_manage.mako' - default_filter = { "openid" : "All" } - default_sort_key = "-create_time" - columns = [ - grids.TextColumn( "OpenID URL", key="openid", link=( lambda x: dict( action='openid_auth', login_button="Login", openid_url=x.openid if not x.provider else '', openid_provider=x.provider, auto_associate=True ) ) ), - grids.GridColumn( "Created", key="create_time", format=time_ago ), - ] - operations = [ - grids.GridOperation( "Delete", async_compatible=True ), - ] - def build_initial_query( self, trans, **kwd ): - return trans.sa_session.query( self.model_class ).filter( self.model_class.user_id == trans.user.id ) +# FIXME: This controller is using unencoded IDs, but I am not going to address +# this now since it is admin-side and should be reimplemented in the API +# anyway. + class User( BaseUIController, UsesFormDefinitionsMixin ): - user_openid_grid = UserOpenIDGrid() - installed_len_files = None - - @web.expose @web.require_login() @web.require_admin def index( self, trans, cntrller, **kwd ): return trans.fill_template( 'webapps/galaxy/user/list_users.mako', action='all_users', cntrller=cntrller ) - - @web.expose @web.require_login() @web.require_admin def admin_api_keys( self, trans, cntrller, uid, **kwd ): params = util.Params( kwd ) - message = util.restore_text( params.get( 'message', '' ) ) + message = escape( util.restore_text( params.get( 'message', '' ) ) ) status = params.get( 'status', 'done' ) uid = params.get('uid', uid) - pprint.pprint(uid) if params.get( 'new_api_key_button', False ): new_key = trans.app.model.APIKeys() new_key.user_id = uid new_key.key = trans.app.security.get_new_guid() trans.sa_session.add( new_key ) trans.sa_session.flush() - message = "Generated a new web API key" + message = "A new web API key has been generated for (%s)" % escape( new_key.user.email ) status = "done" - return trans.fill_template( 'webapps/galaxy/user/ok_admin_api_keys.mako', - cntrller=cntrller, - message=message, - status=status ) - - + return trans.response.send_redirect( web.url_for( controller='userskeys', + action='all_users', + cntrller=cntrller, + message=message, + status=status ) ) + @web.expose @web.require_login() @web.require_admin def all_users( self, trans, cntrller="userskeys", **kwd ): params = util.Params( kwd ) - message = util.restore_text( params.get( 'message', '' ) ) + message = escape( util.restore_text( params.get( 'message', '' ) ) ) status = params.get( 'status', 'done' ) users = [] for user in trans.sa_session.query( trans.app.model.User ) \ diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 templates/user/edit_address.mako --- a/templates/user/edit_address.mako +++ b/templates/user/edit_address.mako @@ -20,7 +20,7 @@ <div class="form-row"><label>Short Description:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="short_desc" value="${address_obj.desc}" size="40"> + <input type="text" name="short_desc" value="${address_obj.desc | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -28,7 +28,7 @@ <div class="form-row"><label>Name:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="name" value="${address_obj.name}" size="40"> + <input type="text" name="name" value="${address_obj.name | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -36,7 +36,7 @@ <div class="form-row"><label>Institution:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="institution" value="${address_obj.institution}" size="40"> + <input type="text" name="institution" value="${address_obj.institution | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -44,7 +44,7 @@ <div class="form-row"><label>Address:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="address" value="${address_obj.address}" size="40"> + <input type="text" name="address" value="${address_obj.address | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -52,7 +52,7 @@ <div class="form-row"><label>City:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="city" value="${address_obj.city}" size="40"> + <input type="text" name="city" value="${address_obj.city | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -60,7 +60,7 @@ <div class="form-row"><label>State/Province/Region:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="state" value="${address_obj.state}" size="40"> + <input type="text" name="state" value="${address_obj.state | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -68,7 +68,7 @@ <div class="form-row"><label>Postal Code:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="postal_code" value="${address_obj.postal_code}" size="40"> + <input type="text" name="postal_code" value="${address_obj.postal_code | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -76,7 +76,7 @@ <div class="form-row"><label>Country:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="country" value="${address_obj.country}" size="40"> + <input type="text" name="country" value="${address_obj.country | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -84,7 +84,7 @@ <div class="form-row"><label>Phone:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="phone" value="${address_obj.phone}" size="40"> + <input type="text" name="phone" value="${address_obj.phone | h}" size="40"></div><div style="clear: both"></div></div> diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 templates/user/index.mako --- a/templates/user/index.mako +++ b/templates/user/index.mako @@ -1,9 +1,4 @@ <%inherit file="/base.mako"/> -<%namespace file="/message.mako" import="render_msg" /> - -%if message: - ${render_msg( message, status )} -%endif %if trans.user: <h2>${_('User preferences')}</h2> diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 templates/user/info.mako --- a/templates/user/info.mako +++ b/templates/user/info.mako @@ -16,19 +16,19 @@ <div class="toolFormTitle">Login Information</div><div class="form-row"><label>Email address:</label> - <input type="text" name="email" value="${email}" size="40"/> + <input type="text" name="email" value="${email | h}" size="40"/></div><div class="form-row"><label>Public name:</label> %if t.webapp.name == 'tool_shed': %if user.active_repositories: - <input type="hidden" name="username" value="${username}"/> - ${username} + <input type="hidden" name="username" value="${username | h}"/> + ${username | h} <div class="toolParamHelp" style="clear: both;"> You cannot change your public name after you have created a repository in this tool shed. </div> %else: - <input type="text" name="username" size="40" value="${username}"/> + <input type="text" name="username" size="40" value="${username | h}"/><div class="toolParamHelp" style="clear: both;"> Your public name provides a means of identifying you publicly within this tool shed. Public names must be at least four characters in length and contain only lower-case letters, numbers, @@ -37,7 +37,7 @@ </div> %endif %else: - <input type="text" name="username" size="40" value="${username}"/> + <input type="text" name="username" size="40" value="${username | h}"/><div class="toolParamHelp" style="clear: both;"> Your public name is an optional identifier that will be used to generate addresses for information you share publicly. Public names must be at least four characters in length and contain only lower-case diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 templates/webapps/galaxy/user/list_users.mako --- a/templates/webapps/galaxy/user/list_users.mako +++ b/templates/webapps/galaxy/user/list_users.mako @@ -1,4 +1,5 @@ <%inherit file="/base.mako"/> +<%namespace file="/message.mako" import="render_msg" /> %if message: ${render_msg( message, status )} diff -r 26aab19109ce7956f29bfc4f5877e6950c0fae56 -r b0d60614d255354f0be66a0ae2434165f5406cb8 templates/webapps/galaxy/user/ok_admin_api_keys.mako --- a/templates/webapps/galaxy/user/ok_admin_api_keys.mako +++ /dev/null @@ -1,28 +0,0 @@ -<%inherit file="/base.mako"/> -<%namespace file="/message.mako" import="render_msg" /> - -<br/><br/> -<ul class="manage-table-actions"> - <li> - <a class="action-button" href="${h.url_for( controller='userskeys', action='all_users', cntrller=cntrller )}">List users API keys</a> - </li> -</ul> - -%if message: - ${render_msg( message, status )} -%endif - - <div> - <div style="clear: both;"> - SUCCESS. A new API key has been generated. - </div> - - - <div style="clear: both;"> - An API key will allow you to access Galaxy via its web - API (documentation forthcoming). Please note that - <strong>this key acts as an alternate means to access - your account, and should be treated with the same care - as your login password</strong>. - </div> - </div> https://bitbucket.org/galaxy/galaxy-central/commits/eb0a5dcc9d63/ Changeset: eb0a5dcc9d63 Branch: stable User: natefoo Date: 2014-12-02 17:06:24+00:00 Summary: Fix various bugs and security (XSS and other) issues with user address handling. Affected #: 4 files diff -r b0d60614d255354f0be66a0ae2434165f5406cb8 -r eb0a5dcc9d633fa8122cca5525a6beb1d9940e62 lib/galaxy/webapps/galaxy/controllers/user.py --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -451,6 +451,7 @@ def login( self, trans, refresh_frames=[], **kwd ): '''Handle Galaxy Log in''' redirect = self.__get_redirect_url( kwd.get( 'redirect', trans.request.referer ).strip() ) + redirect_url = '' # always start with redirect_url being empty use_panels = util.string_as_bool( kwd.get( 'use_panels', False ) ) message = kwd.get( 'message', '' ) status = kwd.get( 'status', 'done' ) @@ -1346,17 +1347,20 @@ # User not logged in, history group must be only public return trans.show_error_message( "You must be logged in to change your default permitted actions." ) + @web.require_login( "to add addresses" ) @web.expose def new_address( self, trans, cntrller, **kwd ): params = util.Params( kwd ) message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) is_admin = cntrller == 'admin' and trans.user_is_admin() - user_id = params.get( 'user_id', False ) - if not user_id: - # User must be logged in to create a new address - return trans.show_error_message( "You must be logged in to create a new address." ) - user = trans.sa_session.query( trans.app.model.User ).get( trans.security.decode_id( user_id ) ) + user_id = params.get( 'id', False ) + if is_admin: + if not user_id: + return trans.show_error_message( "You must specify a user to add a new address to." ) + user = trans.sa_session.query( trans.app.model.User ).get( trans.security.decode_id( user_id ) ) + else: + user = trans.user short_desc = util.restore_text( params.get( 'short_desc', '' ) ) name = util.restore_text( params.get( 'name', '' ) ) institution = util.restore_text( params.get( 'institution', '' ) ) @@ -1407,10 +1411,10 @@ phone=phone ) trans.sa_session.add( user_address ) trans.sa_session.flush() - message = 'Address (%s) has been added' % user_address.desc + message = 'Address (%s) has been added' % escape( user_address.desc ) new_kwd = dict( message=message, status=status ) if is_admin: - new_kwd[ 'user_id' ] = trans.security.encode_id( user.id ) + new_kwd[ 'id' ] = trans.security.encode_id( user.id ) return trans.response.send_redirect( web.url_for( controller='user', action='manage_user_info', cntrller=cntrller, @@ -1428,24 +1432,29 @@ postal_code=postal_code, country=country, phone=phone, - message=message, + message=escape(message), status=status ) + @web.require_login( "to edit addresses" ) @web.expose def edit_address( self, trans, cntrller, **kwd ): params = util.Params( kwd ) - message = escape( util.restore_text( params.get( 'message', '' ) ) ) + message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) is_admin = cntrller == 'admin' and trans.user_is_admin() - user_id = params.get( 'user_id', False ) - if not user_id: - # User must be logged in to create a new address - return trans.show_error_message( "You must be logged in to create a new address." ) - user = trans.sa_session.query( trans.app.model.User ).get( trans.security.decode_id( user_id ) ) + user_id = params.get( 'id', False ) + if is_admin: + if not user_id: + return trans.show_error_message( "You must specify a user to add a new address to." ) + user = trans.sa_session.query( trans.app.model.User ).get( trans.security.decode_id( user_id ) ) + else: + user = trans.user address_id = params.get( 'address_id', None ) if not address_id: - return trans.show_error_message( "No address id received for editing." ) + return trans.show_error_message( "Invalid address id." ) address_obj = trans.sa_session.query( trans.app.model.UserAddress ).get( trans.security.decode_id( address_id ) ) + if address_obj.user_id != user.id: + return trans.show_error_message( "Invalid address id." ) if params.get( 'edit_address_button', False ): short_desc = util.restore_text( params.get( 'short_desc', '' ) ) name = util.restore_text( params.get( 'name', '' ) ) @@ -1493,10 +1502,10 @@ address_obj.phone = phone trans.sa_session.add( address_obj ) trans.sa_session.flush() - message = 'Address (%s) has been updated.' % address_obj.desc + message = 'Address (%s) has been updated.' % escape( address_obj.desc ) new_kwd = dict( message=message, status=status ) if is_admin: - new_kwd[ 'user_id' ] = trans.security.encode_id( user.id ) + new_kwd[ 'id' ] = trans.security.encode_id( user.id ) return trans.response.send_redirect( web.url_for( controller='user', action='manage_user_info', cntrller=cntrller, @@ -1506,45 +1515,44 @@ cntrller=cntrller, user=user, address_obj=address_obj, - message=message, + message=escape( message ), status=status ) + @web.require_login( "to delete addresses" ) @web.expose - def delete_address( self, trans, cntrller, address_id=None, user_id=None ): + def delete_address( self, trans, cntrller, address_id=None, **kwd ): + return self.__delete_undelete_address( trans, cntrller, 'delete', address_id=address_id, **kwd ) + + @web.require_login( "to undelete addresses" ) + @web.expose + def undelete_address( self, trans, cntrller, address_id=None, **kwd ): + return self.__delete_undelete_address( trans, cntrller, 'undelete', address_id=address_id, **kwd ) + + def __delete_undelete_address( self, trans, cntrller, op, address_id=None, **kwd ): + is_admin = cntrller == 'admin' and trans.user_is_admin() + user_id = kwd.get( 'id', False ) + if is_admin: + if not user_id: + return trans.show_error_message( "You must specify a user to %s an address from." % op ) + user = trans.sa_session.query( trans.app.model.User ).get( trans.security.decode_id( user_id ) ) + else: + user = trans.user try: user_address = trans.sa_session.query( trans.app.model.UserAddress ).get( trans.security.decode_id( address_id ) ) except: - message = 'Invalid address is (%s)' % address_id - status = 'error' + return trans.show_error_message( "Invalid address id." ) if user_address: - user_address.deleted = True + if user_address.user_id != user.id: + return trans.show_error_message( "Invalid address id." ) + user_address.deleted = True if op == 'delete' else False trans.sa_session.add( user_address ) trans.sa_session.flush() - message = 'Address (%s) deleted' % user_address.desc + message = 'Address (%s) %sd' % ( escape( user_address.desc ), op ) status = 'done' return trans.response.send_redirect( web.url_for( controller='user', action='manage_user_info', cntrller=cntrller, - user_id=user_id, - message=message, - status=status ) ) - - @web.expose - def undelete_address( self, trans, cntrller, address_id=None, user_id=None ): - try: - user_address = trans.sa_session.query( trans.app.model.UserAddress ).get( trans.security.decode_id( address_id ) ) - except: - message = 'Invalid address is (%s)' % address_id - status = 'error' - if user_address: - user_address.deleted = False - trans.sa_session.flush() - message = 'Address (%s) undeleted' % user_address.desc - status = 'done' - return trans.response.send_redirect( web.url_for( controller='user', - action='manage_user_info', - cntrller=cntrller, - user_id=user_id, + id=trans.security.encode_id( user.id ), message=message, status=status ) ) diff -r b0d60614d255354f0be66a0ae2434165f5406cb8 -r eb0a5dcc9d633fa8122cca5525a6beb1d9940e62 templates/user/edit_address.mako --- a/templates/user/edit_address.mako +++ b/templates/user/edit_address.mako @@ -10,13 +10,13 @@ <ul class="manage-table-actions"><li> - <a class="action-button" href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, user_id=trans.security.encode_id( user.id) )}">Manage user information</a> + <a class="action-button" href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, id=trans.security.encode_id( user.id) )}">Manage user information</a></li></ul><div class="toolForm"><div class="toolFormTitle">Edit address</div><div class="toolFormBody"> - <form name="login_info" id="login_info" action="${h.url_for( controller='user', action='edit_address', cntrller=cntrller, address_id=trans.security.encode_id( address_obj.id ), user_id=trans.security.encode_id( user.id ) )}" method="post" > + <form name="login_info" id="login_info" action="${h.url_for( controller='user', action='edit_address', cntrller=cntrller, address_id=trans.security.encode_id( address_obj.id ), id=trans.security.encode_id( user.id ) )}" method="post" ><div class="form-row"><label>Short Description:</label><div style="float: left; width: 250px; margin-right: 10px;"> diff -r b0d60614d255354f0be66a0ae2434165f5406cb8 -r eb0a5dcc9d633fa8122cca5525a6beb1d9940e62 templates/user/new_address.mako --- a/templates/user/new_address.mako +++ b/templates/user/new_address.mako @@ -10,14 +10,14 @@ <ul class="manage-table-actions"><li> - <a class="action-button" href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, user_id=trans.security.encode_id( user.id) )}"> + <a class="action-button" href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, id=trans.security.encode_id( user.id) )}"><span>Manage User Information</span></a></li></ul><div class="toolForm"><div class="toolFormTitle">Add new address</div><div class="toolFormBody"> - <form name="login_info" id="login_info" action="${h.url_for( controller='user', action='new_address', cntrller=cntrller, user_id=trans.security.encode_id( user.id ) )}" method="post" > + <form name="login_info" id="login_info" action="${h.url_for( controller='user', action='new_address', cntrller=cntrller, id=trans.security.encode_id( user.id ) )}" method="post" ><div class="form-row"><label>Short Description:</label><div style="float: left; width: 250px; margin-right: 10px;"> diff -r b0d60614d255354f0be66a0ae2434165f5406cb8 -r eb0a5dcc9d633fa8122cca5525a6beb1d9940e62 templates/webapps/galaxy/user/manage_info.mako --- a/templates/webapps/galaxy/user/manage_info.mako +++ b/templates/webapps/galaxy/user/manage_info.mako @@ -42,7 +42,7 @@ <p/><div class="toolForm"> - <form name="user_addresses" id="user_addresses" action="${h.url_for( controller='user', action='new_address', cntrller=cntrller, user_id=trans.security.encode_id( user.id ) )}" method="post" > + <form name="user_addresses" id="user_addresses" action="${h.url_for( controller='user', action='new_address', cntrller=cntrller, id=trans.security.encode_id( user.id ) )}" method="post" ><div class="toolFormTitle">User Addresses</div><div class="toolFormBody"> %if user.addresses: @@ -53,9 +53,9 @@ <span>|</span> %endif %if show_filter == filter: - <span class="filter"><a href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, show_filter=filter, user_id=trans.security.encode_id( user.id ) )}"><b>${filter}</b></a></span> + <span class="filter"><a href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, show_filter=filter, id=trans.security.encode_id( user.id ) )}"><b>${filter}</b></a></span> %else: - <span class="filter"><a href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, show_filter=filter, user_id=trans.security.encode_id( user.id ) )}">${filter}</a></span> + <span class="filter"><a href="${h.url_for( controller='user', action='manage_user_info', cntrller=cntrller, show_filter=filter, id=trans.security.encode_id( user.id ) )}">${filter}</a></span> %endif %endfor </div> @@ -73,10 +73,10 @@ <ul class="manage-table-actions"><li> %if not address.deleted: - <a class="action-button" href="${h.url_for( controller='user', action='edit_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), user_id=trans.security.encode_id( user.id ) )}">Edit</a> - <a class="action-button" href="${h.url_for( controller='user', action='delete_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), user_id=trans.security.encode_id( user.id ) )}">Delete</a> + <a class="action-button" href="${h.url_for( controller='user', action='edit_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), id=trans.security.encode_id( user.id ) )}">Edit</a> + <a class="action-button" href="${h.url_for( controller='user', action='delete_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), id=trans.security.encode_id( user.id ) )}">Delete</a> %else: - <a class="action-button" href="${h.url_for( controller='user', action='undelete_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), user_id=trans.security.encode_id( user.id ) )}">Undelete</a> + <a class="action-button" href="${h.url_for( controller='user', action='undelete_address', cntrller=cntrller, address_id=trans.security.encode_id( address.id ), id=trans.security.encode_id( user.id ) )}">Undelete</a> %endif </li></ul> https://bitbucket.org/galaxy/galaxy-central/commits/c4276377c37e/ Changeset: c4276377c37e Branch: stable User: natefoo Date: 2014-12-02 17:17:42+00:00 Summary: Escape input values in new user address form. Affected #: 1 file diff -r eb0a5dcc9d633fa8122cca5525a6beb1d9940e62 -r c4276377c37e6ecd1f7b8de39336bde35d302450 templates/user/new_address.mako --- a/templates/user/new_address.mako +++ b/templates/user/new_address.mako @@ -21,7 +21,7 @@ <div class="form-row"><label>Short Description:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="short_desc" value="${short_desc}" size="40"> + <input type="text" name="short_desc" value="${short_desc | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -29,7 +29,7 @@ <div class="form-row"><label>Name:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="name" value="${name}" size="40"> + <input type="text" name="name" value="${name | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -37,7 +37,7 @@ <div class="form-row"><label>Institution:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="institution" value="${institution}" size="40"> + <input type="text" name="institution" value="${institution | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -45,7 +45,7 @@ <div class="form-row"><label>Address:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="address" value="${address}" size="40"> + <input type="text" name="address" value="${address | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -53,7 +53,7 @@ <div class="form-row"><label>City:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="city" value="${city}" size="40"> + <input type="text" name="city" value="${city | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -61,7 +61,7 @@ <div class="form-row"><label>State/Province/Region:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="state" value="${state}" size="40"> + <input type="text" name="state" value="${state | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -69,7 +69,7 @@ <div class="form-row"><label>Postal Code:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="postal_code" value="${postal_code}" size="40"> + <input type="text" name="postal_code" value="${postal_code | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -77,7 +77,7 @@ <div class="form-row"><label>Country:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="country" value="${country}" size="40"> + <input type="text" name="country" value="${country | h}" size="40"></div><div class="toolParamHelp" style="clear: both;">Required</div><div style="clear: both"></div> @@ -85,7 +85,7 @@ <div class="form-row"><label>Phone:</label><div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="phone" value="${phone}" size="40"> + <input type="text" name="phone" value="${phone | h}" size="40"></div><div style="clear: both"></div></div> https://bitbucket.org/galaxy/galaxy-central/commits/546ff6ef27b4/ Changeset: 546ff6ef27b4 Branch: stable User: natefoo Date: 2014-12-02 18:50:32+00:00 Summary: Don't escape full strings containing desired html. Affected #: 1 file diff -r c4276377c37e6ecd1f7b8de39336bde35d302450 -r 546ff6ef27b4b83e26ae228c292fd981173ac550 lib/galaxy/webapps/galaxy/controllers/user.py --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -164,7 +164,7 @@ user_openid.provider = openid_provider if trans.user: if user_openid.user and user_openid.user.id != trans.user.id: - message = escape( "The OpenID <strong>%s</strong> is already associated with another Galaxy account, <strong>%s</strong>. Please disassociate it from that account before attempting to associate it with a new account." % ( display_identifier, user_openid.user.email ) ) + message = "The OpenID <strong>%s</strong> is already associated with another Galaxy account, <strong>%s</strong>. Please disassociate it from that account before attempting to associate it with a new account." % ( escape( display_identifier ), escape( user_openid.user.email ) ) if not trans.user.active and trans.app.config.user_activation_on: # Account activation is ON and the user is INACTIVE. if ( trans.app.config.activation_grace_period != 0 ): # grace period is ON if self.is_outside_grace_period( trans, trans.user.create_time ): # User is outside the grace period. Login is disabled and he will have the activation email resent. @@ -179,17 +179,17 @@ user_openid.session = trans.galaxy_session if not openid_provider_obj.never_associate_with_user: if not auto_associate and ( user_openid.user and user_openid.user.id == trans.user.id ): - message = escape( "The OpenID <strong>%s</strong> is already associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) ) + message = "The OpenID <strong>%s</strong> is already associated with your Galaxy account, <strong>%s</strong>." % ( escape( display_identifier ), escape( trans.user.email ) ) status = "warning" else: - message = escape( "The OpenID <strong>%s</strong> has been associated with your Galaxy account, <strong>%s</strong>." % ( display_identifier, trans.user.email ) ) + message = "The OpenID <strong>%s</strong> has been associated with your Galaxy account, <strong>%s</strong>." % ( escape( display_identifier ), escape( trans.user.email ) ) status = "done" user_openid.user = trans.user trans.sa_session.add( user_openid ) trans.sa_session.flush() trans.log_event( "User associated OpenID: %s" % display_identifier ) else: - message = escape( "The OpenID <strong>%s</strong> cannot be used to log into your Galaxy account, but any post authentication actions have been performed." % ( openid_provider_obj.name ) ) + message = "The OpenID <strong>%s</strong> cannot be used to log into your Galaxy account, but any post authentication actions have been performed." % escape( openid_provider_obj.name ) status = "info" openid_provider_obj.post_authentication( trans, trans.app.openid_manager, info ) if redirect: 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.