From 0e0b773edc85c3e36ae25f38ee929e8a95cc520b Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 21 Sep 2016 13:32:32 +0100 Subject: [PATCH 1/3] Allow '.' and '_' for public names Fix #2593. Also standardise error messages. --- lib/galaxy/security/validate_user_input.py | 37 +++++---- templates/user/info.mako | 8 +- templates/user/register.mako | 88 +++++++++++----------- test/unit/managers/test_UserManager.py | 2 +- 4 files changed, 70 insertions(+), 65 deletions(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index 88c73d66642..c3b927a2d6e 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -9,15 +9,20 @@ import re log = logging.getLogger( __name__ ) -VALID_PUBLICNAME_RE = re.compile( "^[a-z0-9\-]+$" ) -VALID_PUBLICNAME_SUB = re.compile( "[^a-z0-9\-]" ) +# Email validity parameters +VALID_EMAIL_RE = re.compile( "[^@]+@[^@]+\.[^@]+" ) +EMAIL_MAX_LEN = 255 + +# Public name validity parameters PUBLICNAME_MIN_LEN = 3 PUBLICNAME_MAX_LEN = 255 - -# Basic regular expression to check email validity. -VALID_EMAIL_RE = re.compile( "[^@]+@[^@]+\.[^@]+" ) +VALID_PUBLICNAME_RE = re.compile( "^[a-z0-9._\-]+$" ) +VALID_PUBLICNAME_SUB = re.compile( "[^a-z0-9._\-]" ) FILL_CHAR = '-' +# Password validity parameters +PASSWORD_MIN_LEN = 6 + def validate_email( trans, email, user=None, check_dup=True ): """ @@ -27,9 +32,9 @@ def validate_email( trans, email, user=None, check_dup=True ): if user and user.email == email: return message if not( VALID_EMAIL_RE.match( email ) ): - message = "Please enter your real email address." - elif len( email ) > 255: - message = "Email address exceeds maximum allowable length." + message = "The format of the email address is not correct." + elif len( email ) > EMAIL_MAX_LEN: + message = "Email address cannot be more than %d characters in length." % EMAIL_MAX_LEN elif check_dup and trans.sa_session.query( trans.app.model.User ).filter_by( email=email ).first(): message = "User with that email already exists." # If the blacklist is not empty filter out the disposable domains. @@ -48,13 +53,13 @@ def validate_publicname( trans, publicname, user=None ): if user and user.username == publicname: return '' if len( publicname ) < PUBLICNAME_MIN_LEN: - return "Public name must be at least %d characters in length" % ( PUBLICNAME_MIN_LEN ) + return "Public name must be at least %d characters in length." % ( PUBLICNAME_MIN_LEN ) if len( publicname ) > PUBLICNAME_MAX_LEN: - return "Public name cannot be more than %d characters in length" % ( PUBLICNAME_MAX_LEN ) + return "Public name cannot be more than %d characters in length." % ( PUBLICNAME_MAX_LEN ) if not( VALID_PUBLICNAME_RE.match( publicname ) ): - return "Public name must contain only lower-case letters, numbers and '-'" + return "Public name must contain only lower-case letters, numbers, '.', '_' and '-'." if trans.sa_session.query( trans.app.model.User ).filter_by( username=publicname ).first(): - return "Public name is taken; please choose another" + return "Public name is taken; please choose another." return '' @@ -67,15 +72,15 @@ def transform_publicname( trans, publicname, user=None ): elif publicname not in [ 'None', None, '' ]: publicname = publicname.lower() publicname = re.sub( VALID_PUBLICNAME_SUB, FILL_CHAR, publicname ) - publicname = publicname.ljust( 4, FILL_CHAR )[:255] + publicname = publicname.ljust( PUBLICNAME_MIN_LEN + 1, FILL_CHAR )[:PUBLICNAME_MAX_LEN] if not trans.sa_session.query( trans.app.model.User ).filter_by( username=publicname ).first(): return publicname return '' def validate_password( trans, password, confirm ): - if len( password ) < 6: - return "Use a password of at least 6 characters" + if len( password ) < PASSWORD_MIN_LEN: + return "Use a password of at least %d characters." % PASSWORD_MIN_LEN elif password != confirm: - return "Passwords do not match" + return "Passwords don't match." return '' diff --git a/templates/user/info.mako b/templates/user/info.mako index 632ff29490f..130b9fd27e2 100644 --- a/templates/user/info.mako +++ b/templates/user/info.mako @@ -9,7 +9,7 @@ function validateString(test_string, type) { var mail_re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/; - var username_re = /^[a-z0-9\-]{3,255}$/; + var username_re = /^[a-z0-9._\-]{3,255}$/; if (type === 'email') { return mail_re.test(test_string); } else if (type === 'username'){ @@ -45,9 +45,9 @@ original_username = $( '#name_input' ).val(); $( '#login_info' ).bind( 'submit', function( e ) { - var error_text_email= 'Please enter your valid email address.'; - var error_text_email_long= 'Email cannot be more than 255 characters in length.'; - var error_text_username_characters = 'Public name must contain only lowercase letters, numbers and "-". It also has to be shorter than 255 characters but longer than 2.'; + var error_text_email = 'The format of the email address is not correct.'; + var error_text_email_long = 'Email address cannot be more than 255 characters in length.'; + var error_text_username_characters = "Public name must contain only lowercase letters, numbers, '.', '_' and '-'. It also must be between 3 and 255 characters in length."; var email = $( '#email_input' ).val(); var name = $( '#name_input' ).val(); var validForm = true; diff --git a/templates/user/register.mako b/templates/user/register.mako index d3126f6a1df..3500b3d8feb 100644 --- a/templates/user/register.mako +++ b/templates/user/register.mako @@ -31,7 +31,7 @@ def inherit(context):
%if redirect_url: - %elif message: @@ -64,27 +64,27 @@ def inherit(context): %>
@@ -195,10 +195,10 @@ def inherit(context): %endfor %if not user_type_fd_id_select_field: - %endif + %endif %endif
- If you see this, please leave following field blank. + If you see this, please leave following field blank.
@@ -207,7 +207,7 @@ def inherit(context): %if registration_warning_message:
- ${registration_warning_message} + ${registration_warning_message}
%endif
diff --git a/test/unit/managers/test_UserManager.py b/test/unit/managers/test_UserManager.py index c3884d59be0..260a18a23bd 100644 --- a/test/unit/managers/test_UserManager.py +++ b/test/unit/managers/test_UserManager.py @@ -245,7 +245,7 @@ class UserDeserializerTestCase( BaseTestCase ): self.deserializer.deserialize, user, { 'username': 'ed' }, trans=self.trans ) self.assertTrue( 'Public name must be at least' in str( exception ) ) self.assertRaises( base_manager.ModelDeserializingError, self.deserializer.deserialize, - user, { 'username': 'f.d.r.' }, trans=self.trans ) + user, { 'username': 'f,d,r,' }, trans=self.trans ) self.log( "usernames must be unique" ) self.user_manager.create( **user3_data ) From 70db2b7c4ad253b3ce0443d66df3832e4fc2a189 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 21 Sep 2016 13:56:53 +0100 Subject: [PATCH 2/3] Validate public name when auto-registering a user --- lib/galaxy/webapps/galaxy/controllers/mobile.py | 3 ++- lib/galaxy/webapps/galaxy/controllers/user.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/mobile.py b/lib/galaxy/webapps/galaxy/controllers/mobile.py index c18de38f7a2..1065da37c14 100644 --- a/lib/galaxy/webapps/galaxy/controllers/mobile.py +++ b/lib/galaxy/webapps/galaxy/controllers/mobile.py @@ -67,7 +67,8 @@ class Mobile( BaseUIController ): # kwd['email'] = autoreg[1] # kwd['username'] = autoreg[2] # params = util.Params( kwd ) - # message = validate_email( trans, kwd['email'] ) + # message = " ".join( [ validate_email( trans, kwd['email'] ), + # validate_publicname( trans, kwd['username'] ) ] ).rstrip() # if not message: # message, status, user, success = self.__register( trans, 'user', False, **kwd ) # if success: diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 8acb135afb1..fcd41069c97 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -530,7 +530,8 @@ class User( BaseUIController, UsesFormDefinitionsMixin, CreatesUsersMixin, Creat if autoreg[0]: kwd['email'] = autoreg[1] kwd['username'] = autoreg[2] - message = validate_email( trans, kwd['email'] ) # self.__validate( trans, params, email, password, password, username ) + message = " ".join( [ validate_email( trans, kwd['email'] ), + validate_publicname( trans, kwd['username'] ) ] ).rstrip() if not message: message, status, user, success = self.__register( trans, 'user', False, **kwd ) if success: From 8fb3cd9c5bc52495e6055c0a2a5bb4339e31612b Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 21 Sep 2016 11:44:10 -0400 Subject: [PATCH 3/3] propagate allowed changes to hints --- templates/user/info.mako | 10 +++++----- templates/user/register.mako | 8 ++++---- templates/user/username.mako | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/templates/user/info.mako b/templates/user/info.mako index 130b9fd27e2..c7a17eb2dfc 100644 --- a/templates/user/info.mako +++ b/templates/user/info.mako @@ -102,15 +102,15 @@ ${username | h}
- You cannot change your public name after you have created a repository in this tool shed. + 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 + Your public name provides a means of identifying you publicly within this Tool Shed. Public names must be at least three characters in length and contain only lower-case letters, numbers, - and the '-' character. You cannot change your public name after you have created a repository - in this tool shed. + dots, underscores, and dashes ('.', '_', '-'). You cannot change your public name after you have created a repository + in this Tool Shed.
%endif %else: @@ -118,7 +118,7 @@
Your public name is an identifier that will be used to generate addresses for information you share publicly. Public names must be at least three characters in length and contain only lower-case - letters, numbers, and the '-' character. + letters, numbers, dots, underscores, and dashes ('.', '_', '-').
%endif
diff --git a/templates/user/register.mako b/templates/user/register.mako index 3500b3d8feb..7de41b2440c 100644 --- a/templates/user/register.mako +++ b/templates/user/register.mako @@ -153,15 +153,15 @@ def inherit(context): %if t.webapp.name == 'galaxy':
Your public name is an identifier that will be used to generate addresses for information - you share publicly. Public names must be at least three characters in length and contain only lower-case - letters, numbers, and the '-' character. + you share publicly. Public names must be at least three characters in length and contain only + lower-case letters, numbers, dots, underscores, and dashes ('.', '_', '-').
%else:
Your public name provides a means of identifying you publicly within this tool shed. Public names must be at least three characters in length and contain only lower-case letters, numbers, - and the '-' character. You cannot change your public name after you have created a repository - in this tool shed. + dots, underscores, and dashes ('.', '_', '-'). You cannot change your public name after you have + created a repository in this Tool Shed.
%endif
diff --git a/templates/user/username.mako b/templates/user/username.mako index 118b2c6eb05..adb922f8545 100644 --- a/templates/user/username.mako +++ b/templates/user/username.mako @@ -17,7 +17,7 @@
Your public name is an 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 - letters, numbers, and the '-' character. + letters, numbers, dots, underscores, and dashes ('.', '_', '-').