details: http://www.bx.psu.edu/hg/galaxy/rev/3914645ecccb changeset: 3604:3914645ecccb user: Greg Von Kuster <greg@bx.psu.edu> date: Fri Apr 02 15:00:53 2010 -0400 description: 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. diffstat: 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 | 120 +++++++----- templates/webapps/galaxy/base_panels.mako | 10 +- test/base/twilltestcase.py | 82 ++++++-- test/functional/test_admin_features.py | 25 ++- 12 files changed, 301 insertions(+), 387 deletions(-) diffs (969 lines): diff -r 72daa0d41d1b -r 3914645ecccb lib/galaxy/web/controllers/admin.py --- a/lib/galaxy/web/controllers/admin.py Fri Apr 02 14:34:35 2010 -0400 +++ b/lib/galaxy/web/controllers/admin.py Fri Apr 02 15:00:53 2010 -0400 @@ -711,69 +711,6 @@ 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 @@ 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 -r 72daa0d41d1b -r 3914645ecccb lib/galaxy/web/controllers/user.py --- a/lib/galaxy/web/controllers/user.py Fri Apr 02 14:34:35 2010 -0400 +++ b/lib/galaxy/web/controllers/user.py Fri Apr 02 15:00:53 2010 -0400 @@ -26,23 +26,24 @@ 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 @@ else: trans.handle_user_login( user, webapp ) trans.log_event( "User logged in" ) - msg = "You are now logged in as %s.<br>You can <a href='%s'>go back to the page you were visiting</a> or <a href='%s'>go to the home page</a>." % \ + msg = 'You are now logged in as %s.<br>You can <a target="_top" href="%s">go back to the page you were visiting</a> or <a target="_top" href="%s">go to the home page</a>.' % \ ( user.email, referer, url_for( '/' ) ) if trans.app.config.require_login: - msg += ' <a href="%s">Click here</a> 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 += ' <a target="_top" href="%s">Click here</a> 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 @@ # 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.<br>You can log in again, <a href='%s'>go back to the page you were visiting</a> or <a href='%s'>go to the home page</a>." % \ + msg = 'You have been logged out.<br>You can log in again, <a target="_top" href="%s">go back to the page you were visiting</a> or <a target="_top" href="%s">go to the home page</a>.' % \ ( 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 @@ 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.<br><a href='%s'>Return to the home page.</a>" % ( 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.<br><a href='%s'>Return to the home page.</a>" % ( 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.<br><a target="_top" href="%s">Return to the home page.</a>' % ( 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 @@ 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 @@ 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 @@ 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 -r 72daa0d41d1b -r 3914645ecccb lib/galaxy/web/framework/__init__.py --- a/lib/galaxy/web/framework/__init__.py Fri Apr 02 14:34:35 2010 -0400 +++ b/lib/galaxy/web/framework/__init__.py Fri Apr 02 15:00:53 2010 -0400 @@ -72,7 +72,7 @@ return func( self, trans, *args, **kwargs ) else: return trans.show_error_message( - "You must be <a target='_top' href='%s'>logged in</a> to %s</div>." + 'You must be <a target="_top" href="%s">logged in</a> to %s</div>.' % ( url_for( controller='user', action='login' ), verb ), use_panels=use_panels ) return decorator return argcatcher @@ -434,14 +434,19 @@ 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 -r 72daa0d41d1b -r 3914645ecccb templates/admin/user/create.mako --- a/templates/admin/user/create.mako Fri Apr 02 14:34:35 2010 -0400 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,43 +0,0 @@ -<%inherit file="/base.mako"/> -<%namespace file="/message.mako" import="render_msg" /> - -%if msg: - ${render_msg( msg, messagetype )} -%endif - -<div class="toolForm"> - <div class="toolFormTitle">Create user account</div> - <div class="toolFormBody"> - <form name="form" action="${h.url_for( controller='admin', action='create_new_user', email_filter=email_filter )}" method="post" > - <div class="form-row"> - <label>Email address:</label> - <div style="float: left; width: 250px; margin-right: 10px;"> - <input type="text" name="email" value="${email}" size="40"> - </div> - <div style="clear: both"></div> - </div> - <div class="form-row"> - <label>Password:</label> - <div style="float: left; width: 250px; margin-right: 10px;"> - <input type="password" name="password" value="${password}" size="40"> - </div> - <div style="clear: both"></div> - </div> - <div class="form-row"> - <label>Confirm password:</label> - <div style="float: left; width: 250px; margin-right: 10px;"> - <input type="password" name="confirm" value="${confirm}" size="40"> - </div> - <div style="clear: both"></div> - </div> - <div class="form-row"> - <label>Subscribe To Mailing List:</label> - <div style="float: left; width: 250px; margin-right: 10px;"> - <input type="checkbox" name="subscribe" value="${subscribe}" size="40"> - </div> - <div style="clear: both"></div> - </div> - <input type="submit" name="user_create_button" value="Create"> - </form> - </div> -</div> diff -r 72daa0d41d1b -r 3914645ecccb templates/library/common/browse_library.mako --- a/templates/library/common/browse_library.mako Fri Apr 02 14:34:35 2010 -0400 +++ b/templates/library/common/browse_library.mako Fri Apr 02 15:00:53 2010 -0400 @@ -30,7 +30,6 @@ ${render_content()} </div> </div> - ##${render_content()} </%def> ## Render the grid's basic elements. Each of these elements can be subclassed. diff -r 72daa0d41d1b -r 3914645ecccb templates/user/index.mako --- a/templates/user/index.mako Fri Apr 02 14:34:35 2010 -0400 +++ b/templates/user/index.mako Fri Apr 02 15:00:53 2010 -0400 @@ -1,11 +1,13 @@ <%inherit file="/base.mako"/> -<%def name="title()">User preferences</%def> +<%namespace file="/message.mako" import="render_msg" /> +%if msg: + ${render_msg( msg, messagetype )} +%endif -<h2>${_('User preferences')}</h2> - -%if user: - <p>You are currently logged in as ${user.email}.</p> +%if trans.user: + <h2>${_('User preferences')}</h2> + <p>You are currently logged in as ${trans.user.email}.</p> <ul> %if webapp == 'galaxy': <li><a href="${h.url_for( action='show_info' )}">${_('Manage your information')}</a></li> @@ -14,7 +16,9 @@ <li><a href="${h.url_for( action='logout' )}">${_('Logout')}</a></li> </ul> %else: - <p>${n_('You are currently not logged in.')}</p> + %if not msg: + <p>${n_('You are currently not logged in.')}</p> + %endif <ul> <li><a href="${h.url_for( action='login' )}">${_('Login')}</li> <li><a href="${h.url_for( action='create' )}">${_('Register')}</a></li> diff -r 72daa0d41d1b -r 3914645ecccb templates/user/login.mako --- a/templates/user/login.mako Fri Apr 02 14:34:35 2010 -0400 +++ b/templates/user/login.mako Fri Apr 02 15:00:53 2010 -0400 @@ -1,30 +1,39 @@ <%inherit file="/base.mako"/> <%namespace file="/message.mako" import="render_msg" /> -%if msg: +%if redirect_url: + <script type="text/javascript"> + top.location.href = '${redirect_url}'; + </script> +%endif + +%if not redirect_url and msg: ${render_msg( msg, messagetype )} %endif -<div class="toolForm"> - <div class="toolFormTitle">Login</div> + +%if not trans.user: %if header: ${header} %endif - <form name="login" id="login" action="${h.url_for( controller='user', action='login' )}" method="post" > - <div class="form-row"> - <label>Email address:</label> - <input type="text" name="email" value="" size="40"/> - <input type="hidden" name="webapp" value="${webapp}" size="40"/> - <input type="hidden" name="referer" value="${trans.request.referer}" size="40"/> - </div> - <div class="form-row"> - <label>Password:</label> - <input type="password" name="password" value="" size="40"/> - <div class="toolParamHelp" style="clear: both;"> - <a href="${h.url_for( controller='user', action='reset_password', webapp=webapp, use_panels=use_panels )}">Forgot password? Reset here</a> + <div class="toolForm"> + <div class="toolFormTitle">Login</div> + <form name="login" id="login" action="${h.url_for( controller='user', action='login' )}" method="post" > + <div class="form-row"> + <label>Email address:</label> + <input type="text" name="email" value="${email}" size="40"/> + <input type="hidden" name="webapp" value="${webapp}" size="40"/> + <input type="hidden" name="referer" value="${referer}" size="40"/> </div> - </div> - <div class="form-row"> - <input type="submit" name="login_button" value="Login"/> - </div> - </form> -</div> + <div class="form-row"> + <label>Password:</label> + <input type="password" name="password" value="" size="40"/> + <div class="toolParamHelp" style="clear: both;"> + <a href="${h.url_for( controller='user', action='reset_password', webapp=webapp, use_panels=use_panels )}">Forgot password? Reset here</a> + </div> + </div> + <div class="form-row"> + <input type="submit" name="login_button" value="Login"/> + </div> + </form> + </div> +%endif diff -r 72daa0d41d1b -r 3914645ecccb templates/user/logout.mako --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/templates/user/logout.mako Fri Apr 02 15:00:53 2010 -0400 @@ -0,0 +1,6 @@ +<%inherit file="/base.mako"/> +<%namespace file="/message.mako" import="render_msg" /> + +%if msg: + ${render_msg( msg, messagetype )} +%endif diff -r 72daa0d41d1b -r 3914645ecccb templates/user/register.mako --- a/templates/user/register.mako Fri Apr 02 14:34:35 2010 -0400 +++ b/templates/user/register.mako Fri Apr 02 15:00:53 2010 -0400 @@ -1,6 +1,12 @@ <%inherit file="/base.mako"/> <%namespace file="/message.mako" import="render_msg" /> +%if redirect_url: + <script type="text/javascript"> + top.location.href = '${redirect_url}'; + </script> +%endif + <%def name="javascripts()"> ${parent.javascripts()} <script type="text/javascript"> @@ -33,62 +39,70 @@ from galaxy.web.form_builder import CheckboxField subscribe_check_box = CheckboxField( 'subscribe' ) %> -%if msg: +%if not redirect_url and msg: ${render_msg( msg, messagetype )} %endif -<div class="toolForm"> - <form name="registration" id="registration" action="${h.url_for( controller='user', action='create', admin_view=admin_view )}" method="post" > - <div class="toolFormTitle">Create account</div> - <div class="form-row"> - <label>Email address:</label> - <input type="text" name="email" value="${email}" size="40"/> - <input type="hidden" name="webapp" value="${webapp}" size="40"/> - </div> - <div class="form-row"> - <label>Password:</label> - <input type="password" name="password" value="${password}" size="40"/> - </div> - <div class="form-row"> - <label>Confirm password:</label> - <input type="password" name="confirm" value="${confirm}" size="40"/> - </div> - <div class="form-row"> - <label>Public user name:</label> - <input type="text" name="username" size="40" value="${username}"/> - <div class="toolParamHelp" style="clear: both;"> - When you share or publish items, this name is shown as the author. + +## An admin user may be creating a new user account, in which case we want to display the registration form. +## But if the current user is not an admin user, then don't display the registration form. +%if trans.user_is_admin() or not trans.user: + <div class="toolForm"> + <form name="registration" id="registration" action="${h.url_for( controller='user', action='create', admin_view=admin_view )}" method="post" > + <div class="toolFormTitle">Create account</div> + <div class="form-row"> + <label>Email address:</label> + <input type="text" name="email" value="${email}" size="40"/> + <input type="hidden" name="webapp" value="${webapp}" size="40"/> + <input type="hidden" name="referer" value="${referer}" size="40"/> </div> - </div> - <div class="form-row"> - <label>Subscribe to mailing list:</label> - %if subscribe_checked: - <% subscribe_check_box.checked = True %> + <div class="form-row"> + <label>Password:</label> + <input type="password" name="password" value="${password}" size="40"/> + </div> + <div class="form-row"> + <label>Confirm password:</label> + <input type="password" name="confirm" value="${confirm}" size="40"/> + </div> + <div class="form-row"> + <label>Public user name:</label> + <input type="text" name="username" size="40" value="${username}"/> + <div class="toolParamHelp" style="clear: both;"> + Your user name is an optional identifier that will be used to generate addresses for information + you share publicly. User names must be at least four characters in length and contain only lower-case + letters, numbers, and the '-' character. + </div> + </div> + <div class="form-row"> + <label>Subscribe to mailing list:</label> + %if subscribe_checked: + <% subscribe_check_box.checked = True %> + %endif + ${subscribe_check_box.get_html()} + </div> + %if user_info_select: + <div class="form-row"> + <label>User type</label> + ${user_info_select.get_html()} + </div> %endif - ${subscribe_check_box.get_html()} - </div> - %if user_info_select: + %if user_info_form: + %for field in widgets: + <div class="form-row"> + <label>${field['label']}</label> + ${field['widget'].get_html()} + <div class="toolParamHelp" style="clear: both;"> + ${field['helptext']} + </div> + <div style="clear: both"></div> + </div> + %endfor + %if not user_info_select: + <input type="hidden" name="user_info_select" value="${user_info_form.id}"/> + %endif + %endif <div class="form-row"> - <label>User type</label> - ${user_info_select.get_html()} + <input type="submit" name="create_user_button" value="Submit"/> </div> - %endif - %if user_info_form: - %for field in widgets: - <div class="form-row"> - <label>${field['label']}</label> - ${field['widget'].get_html()} - <div class="toolParamHelp" style="clear: both;"> - ${field['helptext']} - </div> - <div style="clear: both"></div> - </div> - %endfor - %if not user_info_select: - <input type="hidden" name="user_info_select" value="${user_info_form.id}"/> - %endif - %endif - <div class="form-row"> - <input type="submit" name="create_user_button" value="Submit"/> - </div> - </form> -</div> + </form> + </div> +%endif diff -r 72daa0d41d1b -r 3914645ecccb templates/webapps/galaxy/base_panels.mako --- a/templates/webapps/galaxy/base_panels.mako Fri Apr 02 14:34:35 2010 -0400 +++ b/templates/webapps/galaxy/base_panels.mako Fri Apr 02 15:00:53 2010 -0400 @@ -100,28 +100,26 @@ %> <div class="submenu"> <ul class="loggedout-only" style="${style1}"> - <li><a href="${h.url_for( controller='/user', action='login' )}">Login</a></li> + <li><a target="galaxy_main" href="${h.url_for( controller='/user', action='login' )}">Login</a></li> %if app.config.allow_user_creation: - <li><a href="${h.url_for( controller='/user', action='create' )}">Register</a></li> + <li><a target="galaxy_main" href="${h.url_for( controller='/user', action='create' )}">Register</a></li> %endif </ul> <ul class="loggedin-only" style="${style2}"> %if app.config.use_remote_user: %if app.config.remote_user_logout_href: - <li><a href="${app.config.remote_user_logout_href}" target="_top">Logout</a></li> + <li><a target="galaxy_main" href="${app.config.remote_user_logout_href}">Logout</a></li> %endif %else: <li>Logged in as <span id="user-email">${user_email}</span></li> <li><a target="galaxy_main" href="${h.url_for( controller='/user', action='index' )}">Preferences</a></li> <% if app.config.require_login: - logout_target = "" logout_url = h.url_for( controller='/root', action='index', m_c='user', m_a='logout' ) else: - logout_target = "" logout_url = h.url_for( controller='/user', action='logout' ) %> - <li><a target="${logout_target}" href="${logout_url}">Logout</a></li> + <li><a target="_top" href="${logout_url}">Logout</a></li> %endif <li><hr style="color: inherit; background-color: gray"/></li> <li><a target="galaxy_main" href="${h.url_for( controller='/history', action='list' )}">Histories</a></li> diff -r 72daa0d41d1b -r 3914645ecccb test/base/twilltestcase.py --- a/test/base/twilltestcase.py Fri Apr 02 14:34:35 2010 -0400 +++ b/test/base/twilltestcase.py Fri Apr 02 15:00:53 2010 -0400 @@ -793,20 +793,39 @@ self.assertTrue( genome_build == dbkey ) # Functions associated with user accounts - def create( self, email='test@bx.psu.edu', password='testuser', username='admin-user', webapp='galaxy' ): + def create( self, email='test@bx.psu.edu', password='testuser', username='admin-user', webapp='galaxy', referer='' ): # HACK: don't use panels because late_javascripts() messes up the twill browser and it # can't find form fields (and hence user can't be logged in). - self.visit_url( "%s/user/create?use_panels=False&webapp=%s" % ( self.url, webapp ) ) + self.visit_url( "%s/user/create?use_panels=False" % self.url ) tc.fv( '1', 'email', email ) + tc.fv( '1', 'webapp', webapp ) + tc.fv( '1', 'referer', referer ) tc.fv( '1', 'password', password ) tc.fv( '1', 'confirm', password ) tc.fv( '1', 'username', username ) tc.submit( 'create_user_button' ) - self.check_page_for_string( "now logged in as %s" % email ) - # Make sure a new private role was created for the user - self.visit_url( "%s/user/set_default_permissions" % self.url ) - self.check_page_for_string( email ) - self.home() + previously_created = False + username_taken = False + invalid_username = False + try: + self.check_page_for_string( "Created new user account" ) + except: + try: + # May have created the account in a previous test run... + self.check_page_for_string( "User with that email already exists" ) + previously_created = True + except: + try: + self.check_page_for_string( 'This user name is not available' ) + username_taken = True + except: + try: + # Note that we're only checking if the usr name is >< 4 chars here... + self.check_page_for_string( 'User name must be at least 4 characters in length' ) + invalid_username = True + except: + pass + return previously_created, username_taken, invalid_username def create_user_with_info( self, email, password, username, user_info_forms, user_info_form_id, user_info_values ): ''' This method registers a new user and also provides use info @@ -815,7 +834,6 @@ self.visit_url( "%s/user/create?user_info_select=%i&admin_view=False&use_panels=False" % ( self.url, user_info_form_id ) ) else: self.visit_url( "%s/user/create?admin_view=False&use_panels=False" % self.url ) - ##print self.write_temp_file( self.last_page() ) self.check_page_for_string( "Create account" ) tc.fv( "1", "email", email ) tc.fv( "1", "password", password ) @@ -906,20 +924,20 @@ self.visit_url( "%s/%s" % ( self.url, url ) ) self.check_page_for_string( 'Default history permissions have been changed.' ) self.home() - def login( self, email='test@bx.psu.edu', password='testuser', username='admin-user', webapp='galaxy' ): + def login( self, email='test@bx.psu.edu', password='testuser', username='admin-user', webapp='galaxy', referer='' ): # test@bx.psu.edu is configured as an admin user - try: - self.create( email=email, password=password, username=username, webapp=webapp ) - except: - self.home() + previously_created, username_taken, invalid_username = \ + self.create( email=email, password=password, username=username, webapp=webapp, referer=referer ) + if previously_created: + # The acount has previously been created, so just login. # HACK: don't use panels because late_javascripts() messes up the twill browser and it # can't find form fields (and hence user can't be logged in). self.visit_url( "%s/user/login?use_panels=False" % self.url ) tc.fv( '1', 'email', email ) + tc.fv( '1', 'webapp', webapp ) + tc.fv( '1', 'referer', referer ) tc.fv( '1', 'password', password ) - tc.submit( 'Login' ) - self.check_page_for_string( "now logged in as %s" %email ) - self.home() + tc.submit( 'login_button' ) def logout( self ): self.home() self.visit_page( "user/logout" ) @@ -1161,23 +1179,41 @@ # Dataset Security stuff # Tests associated with users - def create_new_account_as_admin( self, email='test4@bx.psu.edu', password='testuser', username='regular-user4' ): + def create_new_account_as_admin( self, email='test4@bx.psu.edu', password='testuser', + username='regular-user4', webapp='galaxy', referer='' ): """Create a new account for another user""" + # HACK: don't use panels because late_javascripts() messes up the twill browser and it + # can't find form fields (and hence user can't be logged in). self.visit_url( "%s/user/create?admin_view=True" % self.url ) tc.fv( '1', 'email', email ) + tc.fv( '1', 'webapp', webapp ) + tc.fv( '1', 'referer', referer ) tc.fv( '1', 'password', password ) tc.fv( '1', 'confirm', password ) tc.fv( '1', 'username', username ) tc.submit( 'create_user_button' ) + previously_created = False + username_taken = False + invalid_username = False try: self.check_page_for_string( "Created new user account" ) - previously_created = False except: - # May have created the account in a previous test run... - self.check_page_for_string( "User with that email already exists" ) - previously_created = True - self.home() - return previously_created + try: + # May have created the account in a previous test run... + self.check_page_for_string( "User with that email already exists" ) + previously_created = True + except: + try: + self.check_page_for_string( 'This user name is not available' ) + username_taken = True + except: + try: + # Note that we're only checking if the usr name is >< 4 chars here... + self.check_page_for_string( 'User name must be at least 4 characters in length' ) + invalid_username = True + except: + pass + return previously_created, username_taken, invalid_username def reset_password_as_admin( self, user_id, password='testreset' ): """Reset a user password""" self.home() diff -r 72daa0d41d1b -r 3914645ecccb test/functional/test_admin_features.py --- a/test/functional/test_admin_features.py Fri Apr 02 14:34:35 2010 -0400 +++ b/test/functional/test_admin_features.py Fri Apr 02 15:00:53 2010 -0400 @@ -24,7 +24,30 @@ # Logged in as admin_user email = 'test3@bx.psu.edu' password = 'testuser' - previously_created = self.create_new_account_as_admin( email=email, password=password, username='regular-user3' ) + # Test setting the user name to one that is already taken. Note that the account must not exist in order + # for this test to work as desired, so the email we're passing is important... + previously_created, username_taken, invalid_username = self.create_new_account_as_admin( email='diff@you.com', + password=password, + username='admin-user', + webapp='galaxy', + referer='' ) + if not username_taken: + raise AssertionError, "The user name (%s) is already being used by another user, but no error was displayed" \ + % 'admin-user' + # Test setting the user name to an invalid one. Note that the account must not exist in order + # for this test to work as desired, so the email we're passing is important... + previously_created, username_taken, invalid_username = self.create_new_account_as_admin( email='diff@you.com', + password=password, + username='h', + webapp='galaxy', + referer='' ) + if not invalid_username: + raise AssertionError, "The user name (%s) is is invalid, but no error was displayed" % username + previously_created, username_taken, invalid_username = self.create_new_account_as_admin( email=email, + password=password, + username='regular-user3', + webapp='galaxy', + referer='' ) # Get the user object for later tests global regular_user3 regular_user3 = get_user( email )