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

[MRG] Add refresh token to auth_state if it is available #157

Merged
merged 1 commit into from
Feb 7, 2018

Conversation

betatim
Copy link
Member

@betatim betatim commented Feb 6, 2018

If the OAuth2 server sends a refresh token, add it to the auth_state.

@@ -134,6 +135,7 @@ def authenticate(self, handler, data=None):
'name': resp_json.get(self.username_key),
'auth_state': {
'access_token': access_token,
'refresh_token': refresh_token,
Copy link
Member Author

Choose a reason for hiding this comment

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

do you prefer the key to always be present (like this) but with None as value or only put the key in if the value is not None? I am easy either way.

Copy link
Member

Choose a reason for hiding this comment

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

No preference from me

@betatim
Copy link
Member Author

betatim commented Feb 6, 2018

There is a whole collection of errors like this:

_______________________ ERROR at teardown of test_auth0 ________________________
    def _close():
        io_loop.clear_current()
>       if (not tornado.ioloop.IOLoop.initialized() or
                io_loop is not tornado.ioloop.IOLoop.instance()):
E               AttributeError: type object 'IOLoop' has no attribute 'initialized'
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/pytest_tornado/plugin.py:136: AttributeError

Any ideas what could be causing this? Recent update to tornado?

@minrk minrk merged commit 36951ae into jupyterhub:master Feb 7, 2018
@betatim betatim deleted the refresh-token branch February 7, 2018 13:54
@betatim
Copy link
Member Author

betatim commented Feb 7, 2018

🎉

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