Add the logic to refresh user auth info #6
Conversation
|
I mentioned it somewhere, but for whatever reason forgot to put it in description - sorry:( Yes, this contains the rebase, hence I also submited that one separatly. Only the last commit will be part of this PR once the rebase is merged |
vpavlin
force-pushed
the
feature/refresh-user
branch
from
c0fc030
to
35f5745
Compare
|
@crobby Rebased |
|
I was able to build and image test with the
Removing the JH user group appears to have left the UI in a blank state and any attempts to login/logout will no allow someone to actually login as the same or a different user
|
| await user.stop() | ||
|
|
||
| return user_info | ||
|
|
There was a problem hiding this comment.
@vpavlin who calls refresh_user? seems like this function returns user_info even when None?
There was a problem hiding this comment.
JupyterHub itself calls it regularly based on auth_refresh_age configuration...default is 300s.
It also gets called before spawning new server when refresh_pre_spawn=True which is desired for us
It returns 3 states True (user info is still valid), False (user info is invalid and needs re-authentication), dict (user info needs to be updated with given values).
So when we return None it means user needs to go through auth again.
| ocp_user = await self.fetch(req) #TODO: tornado.httpclient.HTTPClientError: HTTP 401: Unauthorized | ||
| except HTTPError as ex: | ||
| if ex.code == 401: | ||
| return None |
There was a problem hiding this comment.
Is 401 the only HTTP we need to handle, what about 403: Forbidden?
There was a problem hiding this comment.
This call is to get user's own data from OpenShift OAuth API, so it cannot return Forbidden - it can only return the data or 401...or potentially any other error, but that results in exception which is expected
|
Since the blank UI issue was not introduced by this change, I have not objections to merging. Resolving the |
No description provided.