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

[POC] Auth aware profile list #700

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

npapapietro
Copy link

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 jupyterhub/oauthenticator#571.

Changes

  • _options_form_default: Returns callable if self.profile_list is not empty
  • _render_options_form_dynamically: Checks if self.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 configured oauthenticator_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.

@welcome
Copy link

welcome bot commented Feb 8, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

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.
welcome
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! 👋

Welcome to the Jupyter community! 🎉

@npapapietro npapapietro force-pushed the feature/auth-aware-profile-list branch from d098fe9 to 961f4fd Compare February 8, 2023 19:56
@consideRatio
Copy link
Member

@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.

@npapapietro
Copy link
Author

npapapietro commented Feb 8, 2023

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:

  1. Configuration as Code: The 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.
  2. Security: Certain customers of mine have environments that utilize jupyterhub but run on closed or tightly controlled networks/systems with policies that forbid inlining or 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.

  1. One can use duck typing on the self.authenticator to ensure it has the necessary features to perform the check. (What i've done in this initial attempt
  2. Another is to wrap import 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.

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