From dc728ca16fd6a78bc309553f97bb98e13fc9e57e Mon Sep 17 00:00:00 2001 From: Greg Von Kuster Date: Fri, 2 Apr 2010 15:00:53 -0400 Subject: [PATCH] Handle user registration and login in the galaxy_main frame to ensure masthead. After registration or login, use javascript to redirect to the page the user was viewing when they attempted login. If javascript is disabled, a page will be displayed with links to the page they were visiting or the home page. Revamp functional tests to cover new behavior. Fix bugs where the username was not validated when a user registered, and add functional tests to cover. Fix a bug where the user's new session may have set it's history to the user's previous history, even if the history was deleted. Now a new history will be created for the current session instead. Add a new logout.mako template, and remove a create.mako template that was no longer used. --- lib/galaxy/web/controllers/admin.py | 65 +---- lib/galaxy/web/controllers/user.py | 258 +++++++------------ lib/galaxy/web/framework/__init__.py | 11 +- templates/admin/user/create.mako | 43 ---- templates/library/common/browse_library.mako | 1 - templates/user/index.mako | 16 +- templates/user/login.mako | 51 ++-- templates/user/logout.mako | 6 + templates/user/register.mako | 122 +++++---- templates/webapps/galaxy/base_panels.mako | 10 +- test/base/twilltestcase.py | 82 ++++-- test/functional/test_admin_features.py | 25 +- 12 files changed, 302 insertions(+), 388 deletions(-) delete mode 100644 templates/admin/user/create.mako create mode 100644 templates/user/logout.mako diff --git a/lib/galaxy/web/controllers/admin.py b/lib/galaxy/web/controllers/admin.py index b35a9e62ef7..548c57dabab 100644 --- a/lib/galaxy/web/controllers/admin.py +++ b/lib/galaxy/web/controllers/admin.py @@ -711,69 +711,6 @@ class Admin( BaseController ): return trans.response.send_redirect( web.url_for( controller='user', action='create', admin_view=True ) ) - email = '' - password = '' - confirm = '' - subscribe = False - email_filter = kwargs.get( 'email_filter', 'A' ) - if 'user_create_button' in kwargs: - message = '' - status = '' - email = kwargs.get( 'email' , None ) - password = kwargs.get( 'password', None ) - confirm = kwargs.get( 'confirm', None ) - subscribe = kwargs.get( 'subscribe', None ) - if not email: - message = 'Enter a valid email address' - elif not password: - message = 'Enter a valid password' - elif not confirm: - message = 'Confirm the password' - elif len( email ) == 0 or "@" not in email or "." not in email: - message = 'Enter a real email address' - elif len( email) > 255: - message = 'Email address exceeds maximum allowable length' - elif trans.sa_session.query( trans.app.model.User ).filter_by( email=email ).first(): - message = 'User with that email already exists' - elif len( password ) < 6: - message = 'Use a password of at least 6 characters' - elif password != confirm: - message = 'Passwords do not match' - if message: - trans.response.send_redirect( web.url_for( controller='admin', - action='users', - email_filter=email_filter, - message=util.sanitize_text( message ), - status='error' ) ) - else: - user = trans.app.model.User( email=email ) - user.set_password_cleartext( password ) - if trans.app.config.use_remote_user: - user.external = True - trans.sa_session.add( user ) - trans.sa_session.flush() - trans.app.security_agent.create_private_user_role( user ) - trans.app.security_agent.user_set_default_permissions( user, history=False, dataset=False ) - message = 'Created new user account (%s)' % user.email - status = 'done' - #subscribe user to email list - if subscribe: - mail = os.popen( "%s -t" % trans.app.config.sendmail_path, 'w' ) - mail.write( "To: %s\nFrom: %s\nSubject: Join Mailing List\n\nJoin Mailing list." % ( trans.app.config.mailing_join_addr, email ) ) - if mail.close(): - message + ". However, subscribing to the mailing list has failed." - status = 'error' - trans.response.send_redirect( web.url_for( controller='admin', - action='users', - email_filter=email_filter, - message=util.sanitize_text( message ), - status=status ) ) - return trans.fill_template( '/admin/user/create.mako', - email_filter=email_filter, - email=email, - password=password, - confirm=confirm, - subscribe=subscribe ) @web.expose @web.require_admin def reset_user_password( self, trans, **kwd ): @@ -916,7 +853,7 @@ class Admin( BaseController ): status='done' ) ) @web.expose @web.require_admin - def users( self, trans, **kwargs ): + def users( self, trans, **kwargs ): if 'operation' in kwargs: operation = kwargs['operation'].lower() if operation == "roles": diff --git a/lib/galaxy/web/controllers/user.py b/lib/galaxy/web/controllers/user.py index e1edd98e84d..d1551c0267b 100644 --- a/lib/galaxy/web/controllers/user.py +++ b/lib/galaxy/web/controllers/user.py @@ -26,23 +26,24 @@ VALID_USERNAME_RE = re.compile( "^[a-z0-9\-]+$" ) class User( BaseController ): @web.expose def index( self, trans, webapp='galaxy', **kwd ): - return trans.fill_template( '/user/index.mako', user=trans.get_user(), webapp=webapp ) + return trans.fill_template( '/user/index.mako', webapp=webapp ) @web.expose - def login( self, trans, webapp='galaxy', **kwd ): + def login( self, trans, webapp='galaxy', redirect_url='', refresh_frames=[], **kwd ): + referer = kwd.get( 'referer', trans.request.referer ) use_panels = util.string_as_bool( kwd.get( 'use_panels', True ) ) msg = kwd.get( 'msg', '' ) messagetype = kwd.get( 'messagetype', 'done' ) + header = '' + user = None + email = kwd.get( 'email', '' ) if kwd.get( 'login_button', False ): - email = kwd.get( 'email', '' ) password = kwd.get( 'password', '' ) referer = kwd.get( 'referer', '' ) - if webapp == 'galaxy': + if webapp == 'galaxy' and not refresh_frames: if trans.app.config.require_login: refresh_frames = [ 'masthead', 'history', 'tools' ] else: refresh_frames = [ 'masthead', 'history' ] - else: - refresh_frames = [] user = trans.sa_session.query( trans.app.model.User ).filter( trans.app.model.User.table.c.email==email ).first() if not user: msg = "No such user" @@ -59,38 +60,27 @@ class User( BaseController ): else: trans.handle_user_login( user, webapp ) trans.log_event( "User logged in" ) - msg = "You are now logged in as %s.
You can go back to the page you were visiting or go to the home page." % \ + msg = 'You are now logged in as %s.
You can go back to the page you were visiting or go to the home page.' % \ ( user.email, referer, url_for( '/' ) ) if trans.app.config.require_login: - msg += ' Click here to continue to the home page.' % web.url_for( '/static/welcome.html' ) - return trans.response.send_redirect( web.url_for( controller='user', - action='login', - use_panels=use_panels, - msg=msg, - message_type='done' ) ) - if trans.app.config.require_login: + msg += ' Click here to continue to the home page.' % web.url_for( '/static/welcome.html' ) + redirect_url = referer + if not user and trans.app.config.require_login: if trans.app.config.allow_user_creation: - return trans.fill_template( '/user/login.mako', - webapp=webapp, - header=require_login_creation_template % web.url_for( action='create' ), - use_panels=use_panels, - msg=msg, - messagetype=messagetype, - active_view="user" ) + header = require_login_creation_template % web.url_for( action='create' ) else: - return trans.fill_template( '/user/login.mako', - webapp=webapp, - header=require_login_nocreation_template, - use_panels=use_panels, - msg=msg, - messagetype=messagetype, - active_view="user" ) + header = require_login_nocreation_template return trans.fill_template( '/user/login.mako', webapp=webapp, + email=email, + header=header, use_panels=use_panels, + redirect_url=redirect_url, + referer=referer, + refresh_frames=refresh_frames, msg=msg, messagetype=messagetype, - active_view="use" ) + active_view="user" ) @web.expose def logout( self, trans, webapp='galaxy' ): if webapp == 'galaxy': @@ -103,18 +93,18 @@ class User( BaseController ): # Since logging an event requires a session, we'll log prior to ending the session trans.log_event( "User logged out" ) trans.handle_user_logout() - msg = "You have been logged out.
You can log in again, go back to the page you were visiting or go to the home page." % \ + msg = 'You have been logged out.
You can log in again, go back to the page you were visiting or go to the home page.' % \ ( trans.request.referer, url_for( '/' ) ) - return trans.response.send_redirect( web.url_for( controller='user', - action='login', - msg=msg, - message_type='done' ) ) + return trans.fill_template( '/user/logout.mako', + webapp=webapp, + refresh_frames=refresh_frames, + msg=msg, + messagetype='done', + active_view="user" ) @web.expose - def create( self, trans, webapp='galaxy', **kwd ): + def create( self, trans, webapp='galaxy', redirect_url='', refresh_frames=[], **kwd ): params = util.Params( kwd ) - use_panels = kwd.get( 'use_panels', 'True' ) - # Convert use_panels to Boolean. - use_panels = use_panels in [ 'True', 'true', 't', 'T' ] + use_panels = util.string_as_bool( kwd.get( 'use_panels', True ) ) email = util.restore_text( params.get( 'email', '' ) ) # Do not sanitize passwords, so take from kwd # instead of params ( which were sanitized ) @@ -126,123 +116,56 @@ class User( BaseController ): admin_view = util.string_as_bool( params.get( 'admin_view', False ) ) msg = util.restore_text( params.get( 'msg', '' ) ) messagetype = params.get( 'messagetype', 'done' ) - if webapp == 'galaxy': + referer = kwd.get( 'referer', trans.request.referer ) + if not refresh_frames and webapp == 'galaxy': if trans.app.config.require_login: refresh_frames = [ 'masthead', 'history', 'tools' ] else: refresh_frames = [ 'masthead', 'history' ] - else: - refresh_frames = [] + error = '' if not trans.app.config.allow_user_creation and not trans.user_is_admin(): - msg = 'User registration is disabled. Please contact your Galaxy administrator for an account.' - return trans.response.send_redirect( web.url_for( controller='user', - action='create', - webapp=webapp, - email=email, - password=password, - confirm=confirm, - username=username, - subscribe=subscribe, - subscribe_checked=subscribe_checked, - admin_view=admin_view, - use_panels=use_panels, - refresh_frames=refresh_frames, - msg=error, - messagetype='error' ) ) + error = 'User registration is disabled. Please contact your Galaxy administrator for an account.' # Create the user, save all the user info and login to Galaxy - if params.get( 'create_user_button', False ): + elif params.get( 'create_user_button', False ): # Check email and password validity - error = self.__validate( trans, params, email, password, confirm, webapp ) - if error: - return trans.response.send_redirect( web.url_for( controller='user', - action='create', - webapp=webapp, - email=email, - password=password, - confirm=confirm, - username=username, - subscribe=subscribe, - subscribe_checked=subscribe_checked, - admin_view=admin_view, - use_panels=use_panels, - refresh_frames=refresh_frames, - msg=error, - messagetype='error' ) ) - # all the values are valid - user = trans.app.model.User( email=email ) - user.set_password_cleartext( password ) - user.username = username - trans.sa_session.add( user ) - trans.sa_session.flush() - trans.app.security_agent.create_private_user_role( user ) - if webapp == 'galaxy': - # We set default user permissions, before we log in and set the default history permissions - trans.app.security_agent.user_set_default_permissions( user, - default_access_private=trans.app.config.new_user_dataset_access_role_default_private ) - # save user info - self.__save_user_info( trans, user, action='create', new_user=True, **kwd ) - if subscribe_checked: - mail = os.popen( "%s -t" % trans.app.config.sendmail_path, 'w' ) - mail.write( "To: %s\nFrom: %s\nSubject: Join Mailing List\n\nJoin Mailing list." % ( trans.app.config.mailing_join_addr,email ) ) - if mail.close(): - msg = "Now logged in as " + user.email + ". However, subscribing to the mailing list has failed." - return trans.response.send_redirect( web.url_for( controller='user', - action='create', - webapp=webapp, - email=email, - password=password, - confirm=confirm, - username=username, - subscribe=subscribe, - subscribe_checked=subscribe_checked, - admin_view=admin_view, - use_panels=use_panels, - refresh_frames=refresh_frames, - msg=error, - messagetype='warn' ) ) - if not admin_view: - # The handle_user_login() method has a call to the history_set_default_permissions() method - # (needed when logging in with a history), user needs to have default permissions set before logging in - trans.handle_user_login( user, webapp ) - trans.log_event( "User created a new account" ) - trans.log_event( "User logged in" ) - # subscribe user to email list - msg = "Now logged in as %s.
Return to the home page." % ( user.email, url_for( '/' ) ) - return trans.response.send_redirect( web.url_for( controller='user', - action='create', - webapp=webapp, - email=email, - password=password, - confirm=confirm, - username=username, - subscribe=subscribe, - subscribe_checked=subscribe_checked, - admin_view=admin_view, - use_panels=True, - refresh_frames=refresh_frames, - msg=msg, - messagetype='done' ) ) - else: - trans.response.send_redirect( web.url_for( controller='admin', - action='users', - message='Created new user account (%s)' % user.email, - status='done' ) ) - else: - msg = "Now logged in as %s.
Return to the home page." % ( user.email, url_for( '/' ) ) - return trans.response.send_redirect( web.url_for( controller='user', - action='create', - webapp=webapp, - email=email, - password=password, - confirm=confirm, - username=username, - subscribe=subscribe, - subscribe_checked=subscribe_checked, - admin_view=admin_view, - use_panels=False, - refresh_frames=refresh_frames, - msg=error, - messagetype='done' ) ) + error = self.__validate( trans, params, email, password, confirm, username, webapp ) + if not error: + # all the values are valid + user = trans.app.model.User( email=email ) + user.set_password_cleartext( password ) + user.username = username + trans.sa_session.add( user ) + trans.sa_session.flush() + trans.app.security_agent.create_private_user_role( user ) + msg = 'Now logged in as %s.
Return to the home page.' % ( user.email, url_for( '/' ) ) + if webapp == 'galaxy': + # We set default user permissions, before we log in and set the default history permissions + trans.app.security_agent.user_set_default_permissions( user, + default_access_private=trans.app.config.new_user_dataset_access_role_default_private ) + # save user info + self.__save_user_info( trans, user, action='create', new_user=True, **kwd ) + if subscribe_checked: + # subscribe user to email list + mail = os.popen( "%s -t" % trans.app.config.sendmail_path, 'w' ) + mail.write( "To: %s\nFrom: %s\nSubject: Join Mailing List\n\nJoin Mailing list." % ( trans.app.config.mailing_join_addr,email ) ) + if mail.close(): + error = "Now logged in as " + user.email + ". However, subscribing to the mailing list has failed." + if not error and not admin_view: + # The handle_user_login() method has a call to the history_set_default_permissions() method + # (needed when logging in with a history), user needs to have default permissions set before logging in + trans.handle_user_login( user, webapp ) + trans.log_event( "User created a new account" ) + trans.log_event( "User logged in" ) + elif not error: + trans.response.send_redirect( web.url_for( controller='admin', + action='users', + message='Created new user account (%s)' % user.email, + status='done' ) ) + if not error: + redirect_url = referer + if error: + msg=error + messagetype='error' if webapp == 'galaxy': user_info_select, user_info_form, widgets = self.__user_info_ui( trans, **kwd ) else: @@ -261,6 +184,9 @@ class User( BaseController ): widgets=widgets, webapp=webapp, use_panels=use_panels, + referer=referer, + redirect_url=redirect_url, + refresh_frames=refresh_frames, msg=msg, messagetype=messagetype ) def __save_user_info(self, trans, user, action, new_user=True, **kwd): @@ -371,7 +297,7 @@ class User( BaseController ): if len( username ) > 255: return "User name cannot be more than 255 characters in length" if not( VALID_USERNAME_RE.match( username ) ): - return "User name must contain only letters, numbers and '-'" + return "User name must contain only lower-case letters, numbers and '-'" if trans.sa_session.query( trans.app.model.User ).filter_by( username=username ).first(): return "This user name is not available" return None @@ -382,24 +308,24 @@ class User( BaseController ): elif password != confirm: error = "Passwords do not match" return error - def __validate( self, trans, params, email, password, confirm, webapp ): + def __validate( self, trans, params, email, password, confirm, username, webapp ): error = self.__validate_email( trans, email ) - if error: - return error - error = self.__validate_password( trans, password, confirm ) - if error: - return error - if webapp == 'galaxy': - # TODO: the user controller must be decoupled from the model, so this import causes problems. - # The get_all_forms method is used only if Galaxy is the webapp, so it needs to be re-worked - # so that it can be imported with no problems if the controller is not 'galaxy'. - from galaxy.web.controllers.forms import get_all_forms - if len( get_all_forms( trans, - filter=dict( deleted=False ), - form_type=trans.app.model.FormDefinition.types.USER_INFO ) ): - if params.get( 'user_info_select', 'none' ) == 'none': - return 'Select the user type and the user information' - return None + if not error: + error = self.__validate_password( trans, password, confirm ) + if not error and username: + error = self.__validate_username( trans, username ) + if not error: + if webapp == 'galaxy': + # TODO: the user controller must be decoupled from the model, so this import causes problems. + # The get_all_forms method is used only if Galaxy is the webapp, so it needs to be re-worked + # so that it can be imported with no problems if the controller is not 'galaxy'. + from galaxy.web.controllers.forms import get_all_forms + if len( get_all_forms( trans, + filter=dict( deleted=False ), + form_type=trans.app.model.FormDefinition.types.USER_INFO ) ): + if not params.get( 'user_info_select', False ): + return 'Select the user type and the user information' + return error def __user_info_ui(self, trans, user=None, **kwd): ''' This method creates the user type select box & user information form widgets diff --git a/lib/galaxy/web/framework/__init__.py b/lib/galaxy/web/framework/__init__.py index 5643c7a9891..2d6ac72bf98 100644 --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -72,7 +72,7 @@ def require_login( verb="perform this action", use_panels=False ): return func( self, trans, *args, **kwargs ) else: return trans.show_error_message( - "You must be logged in to %s." + 'You must be logged in to %s.' % ( url_for( controller='user', action='login' ), verb ), use_panels=use_panels ) return decorator return argcatcher @@ -434,14 +434,19 @@ class UniverseWebTransaction( base.DefaultWebTransaction ): except: users_last_session = None last_accessed = False - if prev_galaxy_session.current_history and prev_galaxy_session.current_history.datasets: + if prev_galaxy_session.current_history and \ + not prev_galaxy_session.current_history.deleted and \ + prev_galaxy_session.current_history.datasets: if prev_galaxy_session.current_history.user is None or prev_galaxy_session.current_history.user == user: # If the previous galaxy session had a history, associate it with the new # session, but only if it didn't belong to a different user. history = prev_galaxy_session.current_history elif self.galaxy_session.current_history: history = self.galaxy_session.current_history - if not history and users_last_session and users_last_session.current_history: + if not history and \ + users_last_session and \ + users_last_session.current_history and \ + not users_last_session.current_history.deleted: history = users_last_session.current_history elif not history: history = self.get_history( create=True ) diff --git a/templates/admin/user/create.mako b/templates/admin/user/create.mako deleted file mode 100644 index f76a41f9bfd..00000000000 --- a/templates/admin/user/create.mako +++ /dev/null @@ -1,43 +0,0 @@ -<%inherit file="/base.mako"/> -<%namespace file="/message.mako" import="render_msg" /> - -%if msg: - ${render_msg( msg, messagetype )} -%endif - -
-
Create user account
-
-
-
- -
- -
-
-
-
- -
- -
-
-
-
- -
- -
-
-
-
- -
- -
-
-
- -
-
-
diff --git a/templates/library/common/browse_library.mako b/templates/library/common/browse_library.mako index c938047b091..3a1d2493437 100644 --- a/templates/library/common/browse_library.mako +++ b/templates/library/common/browse_library.mako @@ -30,7 +30,6 @@ ${render_content()} - ##${render_content()} ## Render the grid's basic elements. Each of these elements can be subclassed. diff --git a/templates/user/index.mako b/templates/user/index.mako index e7f28c17e33..7a08a35bcc6 100644 --- a/templates/user/index.mako +++ b/templates/user/index.mako @@ -1,11 +1,13 @@ <%inherit file="/base.mako"/> -<%def name="title()">User preferences +<%namespace file="/message.mako" import="render_msg" /> +%if msg: + ${render_msg( msg, messagetype )} +%endif -

${_('User preferences')}

- -%if user: -

You are currently logged in as ${user.email}.

+%if trans.user: +

${_('User preferences')}

+

You are currently logged in as ${trans.user.email}.

%else: -

${n_('You are currently not logged in.')}

+ %if not msg: +

${n_('You are currently not logged in.')}

+ %endif