Skip to content

Commit

Permalink
Some refactoring and updates to authnz error/exception handling.
Browse files Browse the repository at this point in the history
  • Loading branch information
VJalili committed Dec 20, 2017
1 parent be1acdc commit 0c85d90
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
13 changes: 7 additions & 6 deletions lib/galaxy/authnz/managers.py
Expand Up @@ -97,7 +97,7 @@ def _get_authnz_backend(self, provider):
log.exception('An error occurred when loading PSAAuthnz: ', str(e))
return False, str(e), None
else:
msg = 'The requested identity provider `{}` is not a recognized/expected provider'.format(provider)
msg = 'The requested identity provider, `{}`, is not a recognized/expected provider'.format(provider)
log.debug(msg)
return False, msg, None

Expand All @@ -114,7 +114,7 @@ def authenticate(self, provider, trans):
success, message, backend = self._get_authnz_backend(provider)
if success is False:
return False, message, None
return True, "Redirecting to the identity provider `{}` for authentication".format(provider), backend.authenticate(trans)
return True, "Redirecting to the `{}` identity provider for authentication".format(provider), backend.authenticate(trans)
except Exception as e:
msg = 'An error occurred when authenticating a user on `{}` identity provider: {}'.format(provider, str(e))
log.exception(msg)
Expand All @@ -124,12 +124,12 @@ def callback(self, provider, state_token, authz_code, trans, login_redirect_url)
try:
success, message, backend = self._get_authnz_backend(provider)
if success is False:
return False, message, None, None
return False, message, (None, None)
return True, message, backend.callback(state_token, authz_code, trans, login_redirect_url)
except Exception as e:
msg = 'An error occurred when handling callback from provider `{}`; {}'.format(provider, str(e))
msg = 'An error occurred when handling callback from `{}` identity provider; {}'.format(provider, str(e))
log.exception(msg)
return False, msg, None, None
return False, msg, (None, None)

def disconnect(self, provider, trans, disconnect_redirect_url=None):
try:
Expand All @@ -138,6 +138,7 @@ def disconnect(self, provider, trans, disconnect_redirect_url=None):
return False, message, None
return backend.disconnect(provider, trans, disconnect_redirect_url)
except Exception as e:
msg = 'An error occurred when disconnecting authentication with `{}` for user `{}`; {}'.format(provider, trans.user.username, str(e))
msg = 'An error occurred when disconnecting authentication with `{}` identity provider for user `{}`; ' \
'{}'.format(provider, trans.user.username, str(e))
log.exception(msg)
return False, msg, None
20 changes: 11 additions & 9 deletions lib/galaxy/webapps/galaxy/controllers/authnz.py
Expand Up @@ -24,12 +24,12 @@ def callback(self, trans, provider, **kwargs):
if not bool(kwargs):
log.error("OIDC callback received no data for provider `{}` and user `{}`".format(provider, user))
return trans.show_error_message(
'Did not receive any information from identity provider `{}` to complete user `{}` authentication flow.'
' Please try again, and if the problem persists, contact the Galaxy instance admin. Also note that '
'this endpoint is to receive authentication callbacks only, and should not be called/reached by a '
'user.'.format(provider, user))
'Did not receive any information from the `{}` identity provider to complete user `{}` authentication '
'flow. Please try again, and if the problem persists, contact the Galaxy instance admin. Also note '
'that this endpoint is to receive authentication callbacks only, and should not be called/reached by '
'a user.'.format(provider, user))
if 'error' in kwargs:
log.error("Error handling authentication callback from `{}` provider for user `{}` login request."
log.error("Error handling authentication callback from `{}` identity provider for user `{}` login request."
" Error message: {}".format(provider, user, kwargs.get('error', 'None')))
return trans.show_error_message('Failed to handle authentication callback from {}. '
'Please try again, and if the problem persists, contact '
Expand All @@ -41,6 +41,11 @@ def callback(self, trans, provider, **kwargs):
login_redirect_url=url_for('/'))
if success is False:
return trans.show_error_message(message)
user = user if user is not None else trans.user
if user is None:
return trans.show_error_message("An unknown error occurred when handling the callback from `{}` "
"identity provider. Please try again, and if the problem persists, "
"contact the Galaxy instance admin.".format(provider))
trans.handle_user_login(user)
return trans.fill_template('/user/login.mako',
login=user.username,
Expand All @@ -56,7 +61,7 @@ def callback(self, trans, provider, **kwargs):
active_view="user")

@web.expose
@web.require_login("authenticate against Google identity provider")
@web.require_login("authenticate against the selected identity provider")
def disconnect(self, trans, provider, **kwargs):
if trans.user is None:
# Only logged in users are allowed here.
Expand All @@ -65,6 +70,3 @@ def disconnect(self, trans, provider, **kwargs):
trans,
disconnect_redirect_url=url_for('/'))
return trans.response.send_redirect(redirect_url)

# TODO: check for the error: AuthAlreadyAssociated: This google-openidconnect account is already in use.
# it happens when authenticating a user whose previous authentication was disconnected.

0 comments on commit 0c85d90

Please sign in to comment.