Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ltd 5464-key-error-registration #2180

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

depsiatwal
Copy link
Contributor

https://uktrade.atlassian.net/browse/LTD-5464

This is to rectify an error that manifests itself when registering a new user.
If the user takes too long to fill in their name then when we attempt to get the profile from the SSO service it silently fails.
This is because the token used to make this call is very short lived but the session/token is still active hence we don't redirect using the loginmixin.

By redirecting the user to the login screen we force a token refresh and the user will either be directed to login if they don't have an active token or it gets refreshed and they return back to the register-name screen.

Comment on lines +23 to +31
try:
response = client.get(settings.AUTHBROKER_PROFILE_URL)
response.raise_for_status()
except HTTPError as ex:
logger.info(
"Error with SSO getting profile: %s ",
ex,
)
raise ex
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be slightly simplified by doing:

response = client.get(settings.AUTHBROKER_PROFILE_URL)
try:
    response.raise_for_status()
except HTTPError:
    logger.info("Error with SSO", exc_info=True)
    raise

Comment on lines +198 to +200
try:
self.profile = get_profile(self.request.authbroker_client)
except HTTPError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking for the HTTPError here could we actually check to see if the token that the authbroker_client has is still valid?

I believe most tokens have their own timeout associated with them and that means we could differentiate between a timeout on the token and some other kind of error coming from the auth provider.

@kevincarrogan
Copy link
Contributor

This is because the token used to make this call is very short lived but the session/token is still active hence we don't redirect using the loginmixin.

Having looked at the ticket you suggest that this is because the refresh token is timing out, however that's a long-lived token but this comment implies that the token that's failing is a short-lived one (which isn't the refresh token).

What is the thing that's actually timing out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants