From 0e0b773edc85c3e36ae25f38ee929e8a95cc520b Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 21 Sep 2016 13:32:32 +0100 Subject: [PATCH] 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 )