-
Notifications
You must be signed in to change notification settings - Fork 0
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
Redirect user to login page when oauth session expired #84
Redirect user to login page when oauth session expired #84
Conversation
|
||
appbuilder.add_view(views.HQDatasourceView, 'Update HQ Datasource', menu_cond=lambda *_: False) | ||
appbuilder.add_view(views.SelectDomainView, 'Select a Domain', menu_cond=lambda *_: False) | ||
appbuilder.add_api(api.OAuth) | ||
appbuilder.add_api(api.DataSetChangeAPI) | ||
oauth2_server.config_oauth2(app) | ||
|
||
app.register_error_handler(OAuthSessionExpired, hq_domain.oauth_session_expired) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will this cater to "OAuthError"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, I added the OAuthError
to be caught but didn't push it. See bae3f7a. Thanks for reminder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Let me know if you want to test this on staging, I can still replicate it predictably
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, let me know when deployed to staging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkangia It's deployed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ticket
We've been having an issue on CCA where users which are already logged in to CCA could not access the site due to a 500 error. The error can be seen below.
This was likely due to the refresh token having expired.
This PR simply adds a way of catching the
OAuthSessionExpired
if it's raised in any of the views and does the following:A simple test has been added to test this behaviour.