Support native JupyterHub OAuthenticator in 2i2c-managed hubs #706
Conversation
This is so I could set enabled: False
sgibson91
added
🏷️ authentication
Enhancement
…hubs into jupyterhub-oauth-github
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
| @@ -317,8 +314,6 @@ jupyterhub: | |||
| resp['name'] = resp['name'].split('|')[-1] | |||
| return resp | |||
|
|
|||
| c.JupyterHub.authenticator_class = CustomOAuthenticator | |||
|
|
|||
There was a problem hiding this comment.
I do not get this removal... don't you need to keep this in auth0 enabled clusters?
There was a problem hiding this comment.
So it might be that this line
should be the following instead
generated_config['jupyterhub']['hub']['config']['JupyterHub']['authenticator_class'] = 'CustomOAuthenticator'
I wasn't sure and was a bit confused, hence why I asked for my work to be checked in this discussion thread #706 (comment)
There was a problem hiding this comment.
I am not sure the alternative you wrote in the previous comment will work in your pangeo (non-auth0) case.
What about doing all the stuff under 06-custom-authenticator conditional if auth0 is configured? As it is done here with the scratch_bucket?
There was a problem hiding this comment.
So it might be that this line
should be the following instead
generated_config['jupyterhub']['hub']['config']['JupyterHub']['authenticator_class'] = 'CustomOAuthenticator'
@sgibson91, I believe so too!
At some point, I did this chart about how different configs are put together. I'm not sure if it's super accurate now, but for me, at least it provides a visual starting point, though I'm still a bit confused about them (hence, my forever-taking PR with the staff admin users 🙄 )
One thing that's not accurate in that chart (I think), is that although values.yaml template is the first one that gets loaded, , the extra_config in values.yaml is actually the last one that get loaded by the hub. At least that's what I understand from the hub logs first few lines:
Loading extra config: 01-working-dir
Loading extra config: 02-prometheus
Loading extra config: 03-no-setuid
Loading extra config: 04-custom-theme
Loading extra config: 05-custom-admin
Loading extra config: 06-custom-authenticator
Loading extra config: 07-cloud-storage-bucket
I am not sure the alternative you wrote in the previous comment will work in your pangeo (non-auth0) case.
@damianavila, it shouldn't be set for non-auth0 cases.
There was a problem hiding this comment.
Thank you @GeorgianaElena! I will update that line and see what people think. Also very nice chart! 😉
@damianavila for clarity's sake, we now need to dynamically set the value of JupyterHub.authenticator_class based on which authentication method we are using, hence why these values are being removed from the chart. So when using auth0, authenticator_class will be set in the deployer in this block:
And when using the native GitHubOAuthenticator, we set the class in the *.cluster.yaml hub config, as demonstrated in the accompanying docs, e.g. Step 3 here: https://github.com/sgibson91/pilot-hubs/blob/jupyterhub-oauth-github/docs/howto/configure/auth-management.md#native-jupyterhub-oauthenticator-for-github-orgs-and-teams
What about doing all the stuff under 06-custom-authenticator conditional if auth0 is configured? As it is done here with the scratch_bucket?
I am not convinced that method will work. If I understand the code correctly, the highest level config key that get_config reads in is everything under jupyterhub and the auth0.enabled key is much higher in the hierarchy of keys, adjacent to hubs, so I'm not sure the z2jh module can even see it?
Image to clarify what I mean by the hierarchy:
There was a problem hiding this comment.
I updated the authenticator_class for auth0 hubs in b1d6aac
There was a problem hiding this comment.
Hey, thanks @GeorgianaElena and @sgibson91 for the discussion and the clarifications!
I now understand why I did not catch it... I totally missed this line. This is why I proposed the alternative "conditional approach" (without any testing, of course 😉 ).
In fact, I was expecting that auth0 "enabled" property to have some representation in the schema (I guess that is another discussion to have...), and since I did not find it there, and because I did not check some lines above 🤦 , I assumed there was not a conditional approach.
I concur with the last commit you pushed @sgibson91.
There was a problem hiding this comment.
so I'm not sure the z2jh module can even see it?
Yep, you would need to have somehow an auth under custom as well for being able to catch it... but no need to test that now 😜 .
There was a problem hiding this comment.
I think this one is ready!
Thanks, @sgibson91 for the work you have done here and to bear with my comments/questions.
This commit reconstructs PR #706
Summary
This PR is implementing support for the native JupyterHub OAuthenticator to authenticate against GitHub orgs/teams in our hubs. A config matching the thought process laid out here is currently deployed on the Pangeo staging hub and works! 🎉
fixes #625
What has changed?
config/hubs/now allowsauth0block to have additional properties (such asenabled: false) and makes theauth0.connectionfield not required so we don't have to provide it when using JupyterHub OAuthenticatordeployer/hub.py:JupyterHub.authenticator_classas part of the generated configsecrets/config/hubs/*.cluster.yamlfile. If one exists, read it in and parse the config to thehelm upgradecommand. Merging of config happens at the helm level, instead of merging dicts in Python.auth-management.mddocs to add process for enabling JupyterHub OAuthenticator for GitHub orgs and teams.basehub/values.yaml:JupyterHub.authenticator_classhere before, it is now being set in the deployer as part of the generated config (see above)secrets/config/hubs/*.cluster.yamlfilesnameandconfigparameters are allowed under thehubskey. Any other config should live in the main config file underconfig/hubs.