Open npapapietro opened 1 year ago
Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:
@npapapietro can you try to clarify the problem you wish to solve, and the limitations of for example handling management of the profile list using hooks as examplified in the discourse post?
If I can understand that clearly, its far easier to consider the strategy you have come up with on a high level, and then also on a more detailed level looking at code changes.
I have been using the methods described in that post since it was suggest in my customer's jupyterhub installations :). It works well, but I think that pulling the key auth changes into the kubespawner might improve usability, deployment and security of kubernetes jupyterhub installation (and perhaps other installations as well).
Motivation:
kubespawner_override
sets a good precedent on how to define overrides (such as labels, nodeselectors, service accounts, etc) in configuration (ArgoCD or Flux) for helm releases of jupyterhub. Increasing this extra configurability by oauthenticator_override
would allow for auth based profiles by configuration.exec
type behaviors (which is how I'm currently using it with .Values.hub.extraConfig
.This is a QOL feature in my opinion and by no means a hard requirement for most of my jupyterhub installations, but I have bandwidth to take a whack at it and see if the community would benefit from this as well.
Strategy:
Since the base instance of Spawner
takes the Authenticator
instance, I would take advantage of this inside the Kubespawner
class. There are several paths that can be taken here to ensure OAuthenticator
is selected.
self.authenticator
to ensure it has the necessary features to perform the check. (What i've done in this initial attemptimport OAuthenticator
with a try: ... except ImportError
inside the function. This would probably work fine for the environment built by the z2jh releases.The goal I would make is to reduce the amount of upstream/downstream changes to function signatures outside of the sibling PR i've created in OAuthenticator. One of the caveats with this method is not all OAuth providers are created equal. Some (like Gitlab) required calls to the provider (for pagination perhaps?) to retrieve the user group/role/project data needed to make the user_is_authenticated
determination.
Motivaion
The overall goal of this PR is to allow auth logic in KubeSpawner.profile_list depending on the instance of OAuthenticator chosen. This PR is meant to continue the discussions here as well as a sibling PR to https://github.com/jupyterhub/oauthenticator/pull/571.
Changes
_options_form_default
: Returns callable ifself.profile_list
is not empty_render_options_form_dynamically
: Checks ifself.profile_list
is callable, and generates the list, then proceeds to filter out profiles (and profile options) if the authenticator class meets the auth criteria_filter_profile_options_form
: New async function that filters profiles based on user configuredoauthenticator_override
Per reviewer feedback I will proceed with extending some test coverage to this new logic. No auth is currently being tested beyond Dummy, so this will require some review as well.