From 6c82cf3c22445e97621429591e72951a0941104e Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 11 Apr 2017 11:41:25 -0400 Subject: [PATCH 1/5] Remove logout hack -- logout should always be a redirect or full page action, leaving it free of any JS side-effects this would be necessary for. --- templates/user/logout.mako | 8 -------- 1 file changed, 8 deletions(-) diff --git a/templates/user/logout.mako b/templates/user/logout.mako index fe0a0376bb9..3f6c02f7995 100644 --- a/templates/user/logout.mako +++ b/templates/user/logout.mako @@ -39,14 +39,6 @@ def inherit(context): <%def name="body()"> - %if message: ${render_msg( message, status )} %endif From f75a72aee80680537dad802944585d6007d237c9 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 25 Apr 2017 19:00:53 -0400 Subject: [PATCH 2/5] Rework logic for remote_user/session handling. This should fix issues @erasche reported regarding (null)@example.org users. --- .../web/framework/middleware/remoteuser.py | 15 +++++++------ lib/galaxy/web/framework/webapp.py | 22 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/web/framework/middleware/remoteuser.py b/lib/galaxy/web/framework/middleware/remoteuser.py index f363f4e6b98..4867a0be667 100644 --- a/lib/galaxy/web/framework/middleware/remoteuser.py +++ b/lib/galaxy/web/framework/middleware/remoteuser.py @@ -69,9 +69,13 @@ class RemoteUser( object ): assert self.remote_user_header not in environ environ[ self.remote_user_header ] = self.single_user - # Apache sets REMOTE_USER to the string '(null)' when using the - # Rewrite* method for passing REMOTE_USER and a user is - # un-authenticated. Any other possible values need to go here as well. + if environ.get(self.remote_user_header, '(null)').startswith('(null)'): + # Throw away garbage headers. + # Apache sets REMOTE_USER to the string '(null)' when using the + # Rewrite* method for passing REMOTE_USER and a user is not authenticated. + # Any other possible values need to go here as well. + log.debug("Discarding invalid remote user header %s:%s.", self.remote_user_header, environ.get(self.remote_user_header, None)) + environ.pop(self.remote_user_header) if self.remote_user_header in environ: # process remote user with configuration options. if self.normalize_remote_user_email: @@ -126,10 +130,7 @@ class RemoteUser( object ): """ return self.error( start_response, title, message ) - # Apache sets REMOTE_USER to the string '(null)' when using the - # Rewrite* method for passing REMOTE_USER and a user is - # un-authenticated. Any other possible values need to go here as well. - if not environ.get(self.remote_user_header, '(null)').startswith('(null)'): + if environ.get(self.remote_user_header, None): if not environ[ self.remote_user_header ].count( '@' ): if self.maildomain is not None: environ[ self.remote_user_header ] += '@' + self.maildomain diff --git a/lib/galaxy/web/framework/webapp.py b/lib/galaxy/web/framework/webapp.py index 4ef2085c9a7..dfedaec7f57 100644 --- a/lib/galaxy/web/framework/webapp.py +++ b/lib/galaxy/web/framework/webapp.py @@ -385,7 +385,13 @@ class GalaxyWebTransaction( base.DefaultWebTransaction, elif secure_id: # API authentication via active session # Associate user using existing session - self._ensure_valid_session( session_cookie ) + # This will throw an exception under remote auth with anon users. + try: + self._ensure_valid_session( session_cookie ) + except Exception: + log.exception("Exception during Session-based API authentication, this was most likely an attempt to use an anonymous cookie under remote authentication (so, no user), which we don't support.") + self.user = None + self.galaxy_session = None else: # Anonymous API interaction -- anything but @expose_api_anonymous will fail past here. self.user = None @@ -435,17 +441,13 @@ class GalaxyWebTransaction( base.DefaultWebTransaction, # cases won't have a cookie set above, so we need to to check some # things now. if self.app.config.use_remote_user: - # If this is an api request, and they've passed a key, we let this go. - assert self.app.config.remote_user_header in self.environ, \ - "use_remote_user is set but %s header was not provided" % self.app.config.remote_user_header - remote_user_email = self.environ[ self.app.config.remote_user_header ] + remote_user_email = self.environ.get( self.app.config.remote_user_header, None) if galaxy_session: - # An existing session, make sure correct association exists - if galaxy_session.user is None: + if remote_user_email and galaxy_session.user is None: # No user, associate galaxy_session.user = self.get_or_create_remote_user( remote_user_email ) galaxy_session_requires_flush = True - elif (not remote_user_email.startswith('(null)') and # Apache does this, see remoteuser.py + elif (remote_user_email and (galaxy_session.user.email != remote_user_email) and ((not self.app.config.allow_user_impersonation) or (remote_user_email not in self.app.config.admin_users_list))): @@ -456,9 +458,11 @@ class GalaxyWebTransaction( base.DefaultWebTransaction, user_for_new_session = self.get_or_create_remote_user( remote_user_email ) log.warning( "User logged in as '%s' externally, but has a cookie as '%s' invalidating session", remote_user_email, galaxy_session.user.email ) - else: + elif remote_user_email: # No session exists, get/create user for new session user_for_new_session = self.get_or_create_remote_user( remote_user_email ) + if ((galaxy_session and galaxy_session.user is None) and user_for_new_session is None): + raise Exception("Remote Authentication Failure") else: if galaxy_session is not None and galaxy_session.user and galaxy_session.user.external: # Remote user support is not enabled, but there is an existing From 09ba27543b8726fb61cd998bf9614018a9ba7316 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 25 Apr 2017 19:03:14 -0400 Subject: [PATCH 3/5] Slightly adjust logic for discarding headers; this shouldn't try to throw away empty headers anymore. --- lib/galaxy/web/framework/middleware/remoteuser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/web/framework/middleware/remoteuser.py b/lib/galaxy/web/framework/middleware/remoteuser.py index 4867a0be667..f359324452e 100644 --- a/lib/galaxy/web/framework/middleware/remoteuser.py +++ b/lib/galaxy/web/framework/middleware/remoteuser.py @@ -69,7 +69,7 @@ class RemoteUser( object ): assert self.remote_user_header not in environ environ[ self.remote_user_header ] = self.single_user - if environ.get(self.remote_user_header, '(null)').startswith('(null)'): + if environ.get(self.remote_user_header, '').startswith('(null)'): # Throw away garbage headers. # Apache sets REMOTE_USER to the string '(null)' when using the # Rewrite* method for passing REMOTE_USER and a user is not authenticated. From b779977cdc31d50183e7b0f356e9489a43eb755d Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 25 Apr 2017 19:22:00 -0400 Subject: [PATCH 4/5] Slightly improve error message under remote auth with anon session attempt. --- lib/galaxy/web/framework/webapp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/web/framework/webapp.py b/lib/galaxy/web/framework/webapp.py index dfedaec7f57..c274c3cd907 100644 --- a/lib/galaxy/web/framework/webapp.py +++ b/lib/galaxy/web/framework/webapp.py @@ -462,7 +462,7 @@ class GalaxyWebTransaction( base.DefaultWebTransaction, # No session exists, get/create user for new session user_for_new_session = self.get_or_create_remote_user( remote_user_email ) if ((galaxy_session and galaxy_session.user is None) and user_for_new_session is None): - raise Exception("Remote Authentication Failure") + raise Exception("Remote Authentication Failure - user is unknown and/or not supplied.") else: if galaxy_session is not None and galaxy_session.user and galaxy_session.user.external: # Remote user support is not enabled, but there is an existing From eb9a940a2dd69574b5fbaca06ffdc9beea4d4153 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 25 Apr 2017 19:25:55 -0400 Subject: [PATCH 5/5] Put back old hack; necessary for now. --- templates/user/logout.mako | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/templates/user/logout.mako b/templates/user/logout.mako index 3f6c02f7995..fe0a0376bb9 100644 --- a/templates/user/logout.mako +++ b/templates/user/logout.mako @@ -39,6 +39,14 @@ def inherit(context): <%def name="body()"> + %if message: ${render_msg( message, status )} %endif