-
Notifications
You must be signed in to change notification settings - Fork 74
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
client: handles OAuthException #125
client: handles OAuthException #125
Conversation
invenio_oauthclient/views/client.py
Outdated
return current_oauthclient.handlers[remote_app]() | ||
try: | ||
handler = current_oauthclient.handlers[remote_app]() | ||
except RuntimeError: |
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.
Based on the stacktrace you included, shouldn't this exception be more specific (e.g. OAuthException). Also, I'd like to understand why the exception happens - is this only for CERN or is it also for e.g. ORCID? Catching the error at this level could potentially mask quite a lot of errors which would then never be reported, hence I'd prefer to be a bit careful and understand exactly why.
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.
Yes, for some magic reason when I was checking with OAuthException it wasn't working..Now it's ok and amended..
BTW, the response we are getting from CERN OAuth is {u'error': u'invalid_grant'}
, so it might need further investigation from our side and the CERN OAuth side
bdd83f7
to
66644fe
Compare
66644fe
to
a61ad03
Compare
The error seems to be generated by Flask-Oauthlib Can you put a breakpoint there and extract the |
So the issue when you go 'Back' exists because Flask-oauthlib needs the When the Also, @lnielsen @jirikuncar, how does this thing work for the Github contrib. Does it ever go here? |
invenio_oauthclient/views/client.py
Outdated
if e.type == 'invalid_response': | ||
flash('Code returned was invalid', | ||
category='danger') | ||
return redirect('/') |
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.
Maybe we should consider abort
instead of redirect
. It looks like an invalid flow ...
a61ad03
to
0c797e1
Compare
Signed-off-by: Pamfilos Fokianos <pamfilosf@gmail.com>
0c797e1
to
71c6647
Compare
When you login with CERN OAuth and you go 'Back' it throughs an Internal Server Error
catching the exception and redirecting to homepage fixes the issues