From ba13435838a44b30bfb68b9c4eecf83eb2571221 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Mon, 1 Dec 2014 17:38:24 -0500 Subject: [PATCH 01/10] none of the requests & forms controllers used escaping so I escaped the variables exclusively in the templates --- templates/admin/forms/create_form.mako | 2 +- .../admin/forms/edit_form_definition.mako | 4 +- templates/admin/request_type/common.mako | 4 +- .../request_type/create_request_type.mako | 2 +- .../admin/request_type/edit_request_type.mako | 14 +++--- .../request_type_permissions.mako | 8 ++-- .../admin/request_type/view_request_type.mako | 18 ++++---- templates/admin/requests/reject.mako | 2 +- templates/admin/requests/rename_datasets.mako | 4 +- .../admin/requests/view_sample_dataset.mako | 6 +-- .../galaxy/requests/common/common.mako | 46 +++++++++---------- .../requests/common/create_request.mako | 2 +- .../common/edit_basic_request_info.mako | 6 +-- .../galaxy/requests/common/find_samples.mako | 6 +-- .../galaxy/requests/common/view_request.mako | 10 ++-- .../requests/common/view_request_history.mako | 4 +- .../galaxy/requests/common/view_sample.mako | 20 ++++---- .../requests/common/view_sample_history.mako | 8 ++-- 18 files changed, 83 insertions(+), 83 deletions(-) diff --git a/templates/admin/forms/create_form.mako b/templates/admin/forms/create_form.mako index 7cb981bc162..8d55b642457 100644 --- a/templates/admin/forms/create_form.mako +++ b/templates/admin/forms/create_form.mako @@ -9,7 +9,7 @@
Create a new form definition
%for label, input in inputs:
- + ${input.get_html()}
diff --git a/templates/admin/forms/edit_form_definition.mako b/templates/admin/forms/edit_form_definition.mako index c080e843eeb..d4ea953e61d 100644 --- a/templates/admin/forms/edit_form_definition.mako +++ b/templates/admin/forms/edit_form_definition.mako @@ -96,14 +96,14 @@ $(document).ready(function(){
-
Edit form definition "${form_definition.name}" (${form_definition.type})
+
Edit form definition "${form_definition.name | h}" (${form_definition.type | h})
%if response_redirect: %endif %for label, input in form_details:
%if label != 'Type': - + %endif
${input.get_html()} diff --git a/templates/admin/request_type/common.mako b/templates/admin/request_type/common.mako index 01149003ff7..afb28830971 100644 --- a/templates/admin/request_type/common.mako +++ b/templates/admin/request_type/common.mako @@ -2,7 +2,7 @@
- + ## Do not show remove button for the first state %if element_count > 0: @@ -10,7 +10,7 @@
- +
optional
diff --git a/templates/admin/request_type/create_request_type.mako b/templates/admin/request_type/create_request_type.mako index 5809751e179..cd0bfb1da6e 100644 --- a/templates/admin/request_type/create_request_type.mako +++ b/templates/admin/request_type/create_request_type.mako @@ -23,7 +23,7 @@
Create a new request type
%for rt_info in rt_info_widgets:
- +
${rt_info['widget'].get_html()}
diff --git a/templates/admin/request_type/edit_request_type.mako b/templates/admin/request_type/edit_request_type.mako index 88cfd5e84cf..e3444576bfa 100644 --- a/templates/admin/request_type/edit_request_type.mako +++ b/templates/admin/request_type/edit_request_type.mako @@ -32,26 +32,26 @@
-
"Edit ${request_type.name}" request type
+
"Edit ${request_type.name | h}" request type
- +
- +
- ${request_type.request_form.name} + ${request_type.request_form.name | h} ## Hidden field needed by the __save_request_type() method
- ${request_type.sample_form.name} + ${request_type.sample_form.name | h} ## Hidden field needed by the __save_request_type() method
@@ -63,11 +63,11 @@
- +
- +
optional
diff --git a/templates/admin/request_type/request_type_permissions.mako b/templates/admin/request_type/request_type_permissions.mako index 377a31ff96b..8aa486e9b5a 100644 --- a/templates/admin/request_type/request_type_permissions.mako +++ b/templates/admin/request_type/request_type_permissions.mako @@ -48,7 +48,7 @@ %endif
-
Manage access permissions on request type "${request_type.name}"
+
Manage access permissions on request type "${request_type.name | h}"
@@ -65,13 +65,13 @@ in_roles.add( a.role ) out_roles = filter( lambda x: x not in in_roles, all_roles ) %> - ${action.description}

+ ${action.description | h}

Roles associated:

@@ -80,7 +80,7 @@ Roles not associated:

diff --git a/templates/admin/request_type/view_request_type.mako b/templates/admin/request_type/view_request_type.mako index 10a0a763a0c..cc659097f29 100644 --- a/templates/admin/request_type/view_request_type.mako +++ b/templates/admin/request_type/view_request_type.mako @@ -30,24 +30,24 @@ %endif
-
"${request_type.name}" request type
+
"${request_type.name | h}" request type
- ${request_type.name} + ${request_type.name | h}
- ${request_type.desc} + ${request_type.desc | h}
- ${request_type.request_form.name} + ${request_type.request_form.name | h}
- ${request_type.sample_form.name} + ${request_type.sample_form.name | h}

@@ -55,8 +55,8 @@

Sample states defined for this request type
%for state in request_type.states:
- - ${state.desc} + + ${state.desc | h}
%endfor @@ -67,8 +67,8 @@ %if request_type.external_services: %for index, external_service in enumerate( request_type.external_services ):
- - ${external_service.get_external_service_type( trans ).name} + + ${external_service.get_external_service_type( trans ).name | h}
%endfor %else: diff --git a/templates/admin/requests/reject.mako b/templates/admin/requests/reject.mako index 50243700e21..d742b9f7a26 100644 --- a/templates/admin/requests/reject.mako +++ b/templates/admin/requests/reject.mako @@ -15,7 +15,7 @@
-
Reject sequencing request "${request.name}"
+
Reject sequencing request "${request.name | h}"
Rejecting this request will move the request state to Rejected. diff --git a/templates/admin/requests/rename_datasets.mako b/templates/admin/requests/rename_datasets.mako index d849a34cce2..c9d6174abca 100644 --- a/templates/admin/requests/rename_datasets.mako +++ b/templates/admin/requests/rename_datasets.mako @@ -3,7 +3,7 @@ <% from galaxy.webapps.galaxy.controllers.requests_admin import build_rename_datasets_for_sample_select_field %> -

Rename datasets for Sample "${sample.name}"

+

Rename datasets for Sample "${sample.name | h}"

  • Browse datasets
    • @@ -35,7 +35,7 @@ ${rename_datasets_for_sample_select_field.get_html()} - + ${sample_dataset.file_path} diff --git a/templates/admin/requests/view_sample_dataset.mako b/templates/admin/requests/view_sample_dataset.mako index 52b6e9f9e69..6b16824211e 100644 --- a/templates/admin/requests/view_sample_dataset.mako +++ b/templates/admin/requests/view_sample_dataset.mako @@ -21,19 +21,19 @@
-
"${sample.name}" Dataset
+
"${sample.name | h}" Dataset
- ${sample_dataset.name} + ${sample_dataset.name | h}
- ${sample_dataset.external_service.name} (${sample_dataset.external_service.get_external_service_type( trans ).name}) + ${sample_dataset.external_service.name | h} (${sample_dataset.external_service.get_external_service_type( trans ).name | h})
diff --git a/templates/webapps/galaxy/requests/common/common.mako b/templates/webapps/galaxy/requests/common/common.mako index 401ba04057c..dd012a3d03c 100644 --- a/templates/webapps/galaxy/requests/common/common.mako +++ b/templates/webapps/galaxy/requests/common/common.mako @@ -257,18 +257,18 @@ %endif - +
- ${' (required)' } + (required)
%if display_bar_code: %if is_admin and is_submitted: - + %else: - ${sample_widget['bar_code']} - + ${sample_widget['bar_code'] | h} + %endif %endif @@ -416,7 +416,7 @@ transferred_dataset_files = [] %>
%if can_select_datasets: @@ -439,11 +439,11 @@ %endif
%else: - ${sample_widget_name} + ${sample_widget_name | h} %endif %if display_bar_code: - ${sample_widget_bar_code} + ${sample_widget_bar_code | h} %endif %if is_unsubmitted: Unsubmitted @@ -451,12 +451,12 @@ ${render_sample_state( sample )} %endif %if sample_widget_library and library_cntrller is not None: - ${sample_widget_library.name} + ${sample_widget_library.name | h} %else: %endif %if sample_widget_folder: - ${sample_widget_folder.name} + ${sample_widget_folder.name | h} %else: %endif @@ -464,11 +464,11 @@ %if trans.user == sample_widget_history.user: - ${sample_widget_history.name} + ${sample_widget_history.name | h} %else: - ${sample_widget_history.name} + ${sample_widget_history.name | h} %endif %else: @@ -477,11 +477,11 @@ %if trans.user == sample_widget_workflow.stored_workflow.user: - ${sample_widget_workflow.name} + ${sample_widget_workflow.name | h} %else: - ${sample_widget_workflow.name} + ${sample_widget_workflow.name | h} %endif %else: @@ -519,7 +519,7 @@ <%def name="render_sample_form( index, sample_name, sample_values, fields_dict, display_only )"> - ${sample_name} + ${sample_name | h} %for field_index, field in fields_dict.items(): <% field_type = field[ 'type' ] @@ -532,17 +532,17 @@ %if field_type == 'WorkflowField': %if str( field_value ) != 'none': <% workflow = trans.sa_session.query( trans.app.model.StoredWorkflow ).get( int( field_value ) ) %> - ${workflow.name} + ${workflow.name | h} %endif %else: - ${field_value} + ${field_value | h} %endif %else: None %endif %else: %if field_type == 'TextField': - + %elif field_type == 'SelectField': ${request.user.email} (sequencing request owner) + ${request.user.email | h} (sequencing request owner)
- +
Enter one email address per line
diff --git a/templates/webapps/galaxy/requests/common/find_samples.mako b/templates/webapps/galaxy/requests/common/find_samples.mako index 36974dbbb49..02f1c33bc31 100644 --- a/templates/webapps/galaxy/requests/common/find_samples.mako +++ b/templates/webapps/galaxy/requests/common/find_samples.mako @@ -72,7 +72,7 @@ %if samples: %for sample in samples:
- Sample: ${sample.name} | Barcode: ${sample.bar_code}
+ Sample: ${sample.name | h} | Barcode: ${sample.bar_code | h}
%if sample.request.is_new or not sample.state: State: Unsubmitted
%else: @@ -85,10 +85,10 @@ %> Datasets: ${len( sample.datasets )}
%if is_admin: - User: ${sample.request.user.email} + User: ${sample.request.user.email | h} %endif
- Sequencing request: ${sample.request.name} | Type: ${sample.request.type.name} | State: ${sample.request.state} + Sequencing request: ${sample.request.name | h} | Type: ${sample.request.type.name} | State: ${sample.request.state}

diff --git a/templates/webapps/galaxy/requests/common/view_request.mako b/templates/webapps/galaxy/requests/common/view_request.mako index 8ceab0c739a..98fdc644a8b 100644 --- a/templates/webapps/galaxy/requests/common/view_request.mako +++ b/templates/webapps/galaxy/requests/common/view_request.mako @@ -58,7 +58,7 @@ ${render_samples_messages(request, is_admin, is_submitted, message, status)}
-
Sequencing request "${request.name}"
+
Sequencing request "${request.name | h}"
@@ -67,12 +67,12 @@ ${render_samples_messages(request, is_admin, is_submitted, message, status)}
- ${request.desc} + ${request.desc | h}
- ${request.user.email} + ${request.user.email | h}
@@ -94,7 +94,7 @@ ${render_samples_messages(request, is_admin, is_submitted, message, status)} %>
- ${field_value} + ${field_value | h}
%endfor @@ -116,7 +116,7 @@ ${render_samples_messages(request, is_admin, is_submitted, message, status)} else: emails = '' %> - ${emails} + ${emails | h}
diff --git a/templates/webapps/galaxy/requests/common/view_request_history.mako b/templates/webapps/galaxy/requests/common/view_request_history.mako index e46d465a0a6..89faaaafd48 100644 --- a/templates/webapps/galaxy/requests/common/view_request_history.mako +++ b/templates/webapps/galaxy/requests/common/view_request_history.mako @@ -36,7 +36,7 @@ ${render_msg( message, status )} %endif -

History of sequencing request "${request.name}"

+

History of sequencing request "${request.name | h}"

@@ -52,7 +52,7 @@ - + %endfor diff --git a/templates/webapps/galaxy/requests/common/view_sample.mako b/templates/webapps/galaxy/requests/common/view_sample.mako index 93fcb0df7d7..8d4eede464f 100644 --- a/templates/webapps/galaxy/requests/common/view_sample.mako +++ b/templates/webapps/galaxy/requests/common/view_sample.mako @@ -6,7 +6,7 @@ %if external_service:

-
Available External Service Actions for ${sample.name} at ${external_service.name}
+
Available External Service Actions for ${sample.name | h} at ${external_service.name | h}
%for item in external_service.actions: @@ -25,7 +25,7 @@
- ${external_service_group.label} + ${external_service_group.label | h}
@@ -54,7 +54,7 @@ target = 'galaxy_main' %>
- ${external_service_action.label} + ${external_service_action.label | h}
@@ -75,38 +75,38 @@ %endif
-
Sample "${sample.name}"
+
Sample "${sample.name | h}"
- ${sample.name} + ${sample.name | h}
- ${sample.desc} + ${sample.desc | h}
- ${sample.bar_code} + ${sample.bar_code | h}
%if sample.library:
- ${sample.library.name} + ${sample.library.name | h}
- ${sample.folder.name} + ${sample.folder.name | h}
%endif
- ${sample.request.name} + ${sample.request.name | h}
diff --git a/templates/webapps/galaxy/requests/common/view_sample_history.mako b/templates/webapps/galaxy/requests/common/view_sample_history.mako index 196c0be8aea..902fcffecf0 100644 --- a/templates/webapps/galaxy/requests/common/view_sample_history.mako +++ b/templates/webapps/galaxy/requests/common/view_sample_history.mako @@ -12,7 +12,7 @@ ${render_msg( message, status )} %endif -

History of sample "${sample.name}"

+

History of sample "${sample.name | h}"

${event.state} ${time_ago( event.update_time )}${event.comment}${event.comment | h}
@@ -27,10 +27,10 @@ %for event in sample.events: - - + + - + %endfor From ee153cb4f287390b565d1d6fce36b3b6d4719847 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Tue, 2 Dec 2014 10:42:44 -0500 Subject: [PATCH 02/10] 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. --- lib/galaxy/webapps/galaxy/controllers/user.py | 45 +++++++++-------- .../webapps/galaxy/controllers/userskeys.py | 48 ++++++------------- templates/user/edit_address.mako | 18 +++---- templates/user/index.mako | 5 -- templates/user/info.mako | 10 ++-- templates/webapps/galaxy/user/list_users.mako | 1 + .../galaxy/user/ok_admin_api_keys.mako | 28 ----------- 7 files changed, 55 insertions(+), 100 deletions(-) delete mode 100644 templates/webapps/galaxy/user/ok_admin_api_keys.mako diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 07a96bab92b..da52f0c2f64 100644 --- 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 CreatesUsersMixin 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat user_openid.provider = openid_provider if trans.user: if user_openid.user and user_openid.user.id != trans.user.id: - message = "The OpenID %s is already associated with another Galaxy account, %s. 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 %s is already associated with another Galaxy account, %s. 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 %s is already associated with your Galaxy account, %s." % ( display_identifier, trans.user.email ) + message = escape( "The OpenID %s is already associated with your Galaxy account, %s." % ( display_identifier, trans.user.email ) ) status = "warning" else: - message = "The OpenID %s has been associated with your Galaxy account, %s." % ( display_identifier, trans.user.email ) + message = escape( "The OpenID %s has been associated with your Galaxy account, %s." % ( 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 %s 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 %s 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
Click here to return to the page you were previously viewing.' % ( message, redirect ) + message = '%s
Click here 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat @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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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. Click here to return to the login form." % ( email, web.url_for( controller='user', action='login' ) ) + message = "Password has been reset and emailed to: %s. Click here 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat @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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat @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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 --git a/lib/galaxy/webapps/galaxy/controllers/userskeys.py b/lib/galaxy/webapps/galaxy/controllers/userskeys.py index fbcbc3d7b86..4cba3086f12 100644 --- a/lib/galaxy/webapps/galaxy/controllers/userskeys.py +++ b/lib/galaxy/webapps/galaxy/controllers/userskeys.py @@ -3,12 +3,11 @@ Contains the user interface in the Universe class """ 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 @@ require_login_template = """

""" -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 --git a/templates/user/edit_address.mako b/templates/user/edit_address.mako index 430c967e356..e20513ce10b 100644 --- a/templates/user/edit_address.mako +++ b/templates/user/edit_address.mako @@ -20,7 +20,7 @@

- +
Required
@@ -28,7 +28,7 @@
- +
Required
@@ -36,7 +36,7 @@
- +
Required
@@ -44,7 +44,7 @@
- +
Required
@@ -52,7 +52,7 @@
- +
Required
@@ -60,7 +60,7 @@
- +
Required
@@ -68,7 +68,7 @@
- +
Required
@@ -76,7 +76,7 @@
- +
Required
@@ -84,7 +84,7 @@
- +
diff --git a/templates/user/index.mako b/templates/user/index.mako index ed56148b599..df27c61499b 100644 --- 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:

${_('User preferences')}

diff --git a/templates/user/info.mako b/templates/user/info.mako index 3a5896cf6f0..944ebbfc511 100644 --- a/templates/user/info.mako +++ b/templates/user/info.mako @@ -16,19 +16,19 @@
Login Information
- +
%if t.webapp.name == 'tool_shed': %if user.active_repositories: - - ${username} + + ${username | h}
You cannot change your public name after you have created a repository in this tool shed.
%else: - +
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 @@
%endif %else: - +
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 --git a/templates/webapps/galaxy/user/list_users.mako b/templates/webapps/galaxy/user/list_users.mako index b6968c3868c..e5638b9448c 100644 --- 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 --git a/templates/webapps/galaxy/user/ok_admin_api_keys.mako b/templates/webapps/galaxy/user/ok_admin_api_keys.mako deleted file mode 100644 index e7eb97be5a1..00000000000 --- 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" /> - -

- - -%if message: - ${render_msg( message, status )} -%endif - -
-
- SUCCESS. A new API key has been generated. -
- - -
- An API key will allow you to access Galaxy via its web - API (documentation forthcoming). Please note that - this key acts as an alternate means to access - your account, and should be treated with the same care - as your login password. -
-
From c4ad6d7ae6563c0c0ca2e479f0d33db33d45d4f3 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Tue, 2 Dec 2014 12:06:24 -0500 Subject: [PATCH 03/10] Fix various bugs and security (XSS and other) issues with user address handling. --- lib/galaxy/webapps/galaxy/controllers/user.py | 94 ++++++++++--------- templates/user/edit_address.mako | 4 +- templates/user/new_address.mako | 4 +- .../webapps/galaxy/user/manage_info.mako | 12 +-- 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index da52f0c2f64..67260520495 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -451,6 +451,7 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat # 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 --git a/templates/user/edit_address.mako b/templates/user/edit_address.mako index e20513ce10b..6f27d5e1aaa 100644 --- a/templates/user/edit_address.mako +++ b/templates/user/edit_address.mako @@ -10,13 +10,13 @@
Edit address
- +
diff --git a/templates/user/new_address.mako b/templates/user/new_address.mako index f9e8d5e8c63..2acdfb56908 100644 --- a/templates/user/new_address.mako +++ b/templates/user/new_address.mako @@ -10,14 +10,14 @@
Add new address
- +
diff --git a/templates/webapps/galaxy/user/manage_info.mako b/templates/webapps/galaxy/user/manage_info.mako index e3c3f2f399a..8b1a747b662 100644 --- a/templates/webapps/galaxy/user/manage_info.mako +++ b/templates/webapps/galaxy/user/manage_info.mako @@ -42,7 +42,7 @@ ${render_user_info()}

- +
User Addresses
%if user.addresses: @@ -53,9 +53,9 @@ ${render_user_info()} | %endif %if show_filter == filter: - ${filter} + ${filter} %else: - ${filter} + ${filter} %endif %endfor
@@ -73,10 +73,10 @@ ${render_user_info()} From bc9256c3e68d54216663aa9dd1bf21168fdc7050 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Tue, 2 Dec 2014 12:17:42 -0500 Subject: [PATCH 04/10] Escape input values in new user address form. --- templates/user/new_address.mako | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/templates/user/new_address.mako b/templates/user/new_address.mako index 2acdfb56908..c80f613cdcc 100644 --- a/templates/user/new_address.mako +++ b/templates/user/new_address.mako @@ -21,7 +21,7 @@
- +
Required
@@ -29,7 +29,7 @@
- +
Required
@@ -37,7 +37,7 @@
- +
Required
@@ -45,7 +45,7 @@
- +
Required
@@ -53,7 +53,7 @@
- +
Required
@@ -61,7 +61,7 @@
- +
Required
@@ -69,7 +69,7 @@
- +
Required
@@ -77,7 +77,7 @@
- +
Required
@@ -85,7 +85,7 @@
- +
From 53cca8f8f11f988c182461cad0a664954cfdfa06 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Tue, 2 Dec 2014 13:50:32 -0500 Subject: [PATCH 05/10] Don't escape full strings containing desired html. --- lib/galaxy/webapps/galaxy/controllers/user.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 67260520495..0574b5f79f6 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -164,7 +164,7 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat user_openid.provider = openid_provider if trans.user: if user_openid.user and user_openid.user.id != trans.user.id: - message = escape( "The OpenID %s is already associated with another Galaxy account, %s. Please disassociate it from that account before attempting to associate it with a new account." % ( display_identifier, user_openid.user.email ) ) + message = "The OpenID %s is already associated with another Galaxy account, %s. 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 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat 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 %s is already associated with your Galaxy account, %s." % ( display_identifier, trans.user.email ) ) + message = "The OpenID %s is already associated with your Galaxy account, %s." % ( escape( display_identifier ), escape( trans.user.email ) ) status = "warning" else: - message = escape( "The OpenID %s has been associated with your Galaxy account, %s." % ( display_identifier, trans.user.email ) ) + message = "The OpenID %s has been associated with your Galaxy account, %s." % ( 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 %s cannot be used to log into your Galaxy account, but any post authentication actions have been performed." % ( openid_provider_obj.name ) ) + message = "The OpenID %s 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: From b9027d69ef8ee0d654454e9cd24a3f9727acaa01 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Wed, 3 Dec 2014 09:21:54 -0500 Subject: [PATCH 06/10] Bump NO_OUTPUT_TIMEOUT to 60m; Bjoern said it's causing issues w/ some tools for being too short. --- lib/tool_shed/util/basic_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tool_shed/util/basic_util.py b/lib/tool_shed/util/basic_util.py index 58dbb17e564..c11ee1fb971 100644 --- a/lib/tool_shed/util/basic_util.py +++ b/lib/tool_shed/util/basic_util.py @@ -16,7 +16,7 @@ log = logging.getLogger( __name__ ) CHUNK_SIZE = 2**20 # 1Mb INSTALLATION_LOG = 'INSTALLATION.log' # Set no activity timeout to 20 minutes. -NO_OUTPUT_TIMEOUT = 1200.0 +NO_OUTPUT_TIMEOUT = 3600.0 MAXDIFFSIZE = 8000 MAX_DISPLAY_SIZE = 32768 From d1cde6b943b56ee8f209acae8ff9b2708559154e Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Wed, 3 Dec 2014 09:23:14 -0500 Subject: [PATCH 07/10] Pep8 tool_shed/util/basic_util --- lib/tool_shed/util/basic_util.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/tool_shed/util/basic_util.py b/lib/tool_shed/util/basic_util.py index c11ee1fb971..11d2895cedd 100644 --- a/lib/tool_shed/util/basic_util.py +++ b/lib/tool_shed/util/basic_util.py @@ -13,7 +13,7 @@ import markupsafe log = logging.getLogger( __name__ ) -CHUNK_SIZE = 2**20 # 1Mb +CHUNK_SIZE = 2**20 # 1Mb INSTALLATION_LOG = 'INSTALLATION.log' # Set no activity timeout to 20 minutes. NO_OUTPUT_TIMEOUT = 3600.0 @@ -47,6 +47,7 @@ SELECTED_REPOSITORIES_TEMPLATE = ''' RUN service postgresql start && service apache2 start && ./run.sh --daemon && sleep 120 && python ./scripts/api/install_tool_shed_repositories.py --api admin -l http://localhost:8080 --url ${tool_shed_url} -o ${repository_owner} --name ${repository_name} --tool-deps --repository-deps --panel-section-name 'Docker' ''' + def evaluate_template( text, install_environment ): """ Substitute variables defined in XML blocks from dependencies file. The value of the received @@ -56,6 +57,7 @@ def evaluate_template( text, install_environment ): """ return Template( text ).safe_substitute( get_env_var_values( install_environment ) ) + def get_env_var_values( install_environment ): """ Return a dictionary of values, some of which enable substitution of reserved words for the values. @@ -72,6 +74,7 @@ def get_env_var_values( install_environment ): env_var_dict[ '__is64bit__' ] = sys.maxsize > 2**32 return env_var_dict + def get_file_type_str( changeset_revision, file_type ): if file_type == 'zip': file_type_str = '%s.zip' % changeset_revision @@ -83,6 +86,7 @@ def get_file_type_str( changeset_revision, file_type ): file_type_str = '' return file_type_str + def move_file( current_dir, source, destination, rename_to=None ): source_path = os.path.abspath( os.path.join( current_dir, source ) ) source_file = os.path.basename( source_path ) @@ -97,6 +101,7 @@ def move_file( current_dir, source, destination, rename_to=None ): os.makedirs( destination_directory ) shutil.move( source_path, destination_path ) + def remove_dir( dir ): """Attempt to remove a directory from disk.""" if dir: @@ -106,6 +111,7 @@ def remove_dir( dir ): except: pass + def size_string( raw_text, size=MAX_DISPLAY_SIZE ): """Return a subset of a string (up to MAX_DISPLAY_SIZE) translated to a safe string for display in a browser.""" if raw_text and len( raw_text ) >= size: @@ -113,11 +119,13 @@ def size_string( raw_text, size=MAX_DISPLAY_SIZE ): raw_text = '%s%s' % ( raw_text[ 0:size ], large_str ) return raw_text or '' + def stringify( list ): if list: return ','.join( list ) return '' + def strip_path( fpath ): """Attempt to strip the path from a file name.""" if not fpath: @@ -128,6 +136,7 @@ def strip_path( fpath ): file_name = fpath return file_name + def to_html_string( text ): """Translates the characters in text to an html string""" if text: From dfc4e52fd3fbff3add8dfd919425dcd1e3734cac Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Wed, 3 Dec 2014 09:24:44 -0500 Subject: [PATCH 08/10] Fix size_string; this would have thrown an exception on being called due to an unimported 'util'. --- lib/tool_shed/util/basic_util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/tool_shed/util/basic_util.py b/lib/tool_shed/util/basic_util.py index 11d2895cedd..60c7d2ca570 100644 --- a/lib/tool_shed/util/basic_util.py +++ b/lib/tool_shed/util/basic_util.py @@ -4,7 +4,7 @@ import shutil import sys from string import Template -from galaxy.util import unicodify +from galaxy.util import unicodify, nice_size from galaxy import eggs @@ -115,7 +115,7 @@ def remove_dir( dir ): def size_string( raw_text, size=MAX_DISPLAY_SIZE ): """Return a subset of a string (up to MAX_DISPLAY_SIZE) translated to a safe string for display in a browser.""" if raw_text and len( raw_text ) >= size: - large_str = '\nFile contents truncated because file size is larger than maximum viewing size of %s\n' % util.nice_size( size ) + large_str = '\nFile contents truncated because file size is larger than maximum viewing size of %s\n' % nice_size( size ) raw_text = '%s%s' % ( raw_text[ 0:size ], large_str ) return raw_text or '' From e25be8284fc49547e34356778b79a392d138ee43 Mon Sep 17 00:00:00 2001 From: Daniel Blankenberg Date: Wed, 3 Dec 2014 11:35:35 -0500 Subject: [PATCH 09/10] DatasetMatcher should check to see if hda is of the correct format before attempting to filter on e.g. metadata attributes (that may not exist for a non-expected format). --- .../tools/parameters/dataset_matcher.py | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index ea959c4325d..8a12ebaafef 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -39,27 +39,30 @@ class DatasetMatcher( object ): return state_valid and ( not check_security or self.__can_access_dataset( dataset ) ) def valid_hda_match( self, hda, check_implicit_conversions=True, check_security=False ): - """ Return False of this parameter can not be matched to a the supplied + """ Return False of this parameter can not be matched to the supplied HDA, otherwise return a description of the match (either a HdaDirectMatch describing a direct match or a HdaImplicitMatch describing an implicit conversion.) """ - if self.filter( hda ): - return False + rval = False formats = self.param.formats if hda.datatype.matches_any( formats ): - return HdaDirectMatch( hda ) - if not check_implicit_conversions: - return False - target_ext, converted_dataset = hda.find_conversion_destination( formats ) - if target_ext: - original_hda = hda - if converted_dataset: - hda = converted_dataset - if check_security and not self.__can_access_dataset( hda.dataset ): + rval = HdaDirectMatch( hda ) + else: + if not check_implicit_conversions: return False - return HdaImplicitMatch( hda, target_ext, original_hda ) - return False + target_ext, converted_dataset = hda.find_conversion_destination( formats ) + if target_ext: + if converted_dataset: + hda = converted_dataset + if check_security and not self.__can_access_dataset( hda.dataset ): + return False + rval = HdaImplicitMatch( hda, target_ext ) + else: + return False + if self.filter( hda ): + return False + return rval def hda_match( self, hda, check_implicit_conversions=True, ensure_visible=True ): """ If HDA is accessible, return information about whether it could From 4d8e8e877ccbc343c89561aee7fa816986422462 Mon Sep 17 00:00:00 2001 From: Daniel Blankenberg Date: Wed, 3 Dec 2014 12:17:25 -0500 Subject: [PATCH 10/10] Specify the third argument for HdaImplicitMatch. --- lib/galaxy/tools/parameters/dataset_matcher.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index 8a12ebaafef..f378611f1d6 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -53,11 +53,12 @@ class DatasetMatcher( object ): return False target_ext, converted_dataset = hda.find_conversion_destination( formats ) if target_ext: + original_hda = hda if converted_dataset: hda = converted_dataset if check_security and not self.__can_access_dataset( hda.dataset ): return False - rval = HdaImplicitMatch( hda, target_ext ) + rval = HdaImplicitMatch( hda, target_ext, original_hda ) else: return False if self.filter( hda ):
${event.state.name}${event.state.desc}${event.state.name | h}${event.state.desc | h} ${time_ago( event.update_time )}${event.comment}${event.comment | h}