[GitHub] allow union of allowed_users and allowed_organizations

[bolliger32]

Proposed change

We recently updated our jupyterhub helm chart on a z2jh deployment. We've run into issues where external collaborators that are not part of our GitHub org are receiving 403 errors when trying to log back in. I think we've isolated the issue to a change in the way the allowed_users (a.k.a. whitelist) and allowed_organizations (a.k.a. orgWhitelist) are combined. It seems that previously the union of these was used to determine authentication, so that we could put our GitHub org under the orgWhitelist value and then external collaborators under the whitelist value. But now, it looks like the authenticator is implemented such that it is taking the intersection, rather than the union, of those two groups (allowed_organizations is first checked in

oauthenticator/oauthenticator/github.py

Line 180 in be91e7d

if self.allowed_organizations:

and then if it passes, allowed_users is checked...somewhere else down the line I presume?

Ideally, it would be nice to take the union as we used to.

This issue is perhaps similar to #265 ? Though the issue of teams is different than this "individuals" vs. "organizations" issue

Alternative options

I'm not sure of great alternatives... but would be open to any. If there's a way to make our use case work (i.e. taking the union of allowed_users and allowed_organizations) using the existing structure, that would be great and could avoid any new development.

Who would use this feature?

Any use case where there is a central organization (or set of orgs) with individual collaborators that are not part of the GitHub org but need access to the z2jh instance.

(Optional): Suggest a solution

Maybe somewhere around here, you could also check whether a user falls into self.allowed_users? Would that work? It seems that some other adjustments would need to be made (perhaps in the jupyterhub package?) as allowed_users is under the c.Authenticator config and allowed_organizations is under c.GitHubOAuthenticator. But I don't know too much about the structure of these packages so am hoping others might think of a better way to do this. Allowing for both the option to take the intersection or the union of these two filters admittedly seems tricky.

[welcome]

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.

You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[consideRatio]

@bolliger32 what a beautifully written issue!! ❤️ 🎉

What z2jh version upgrade did you make? I will be able to translate it to a change of oauthenticator version. This sound like a regression bug rather than a enhancement proposal, and very relevant to solve in my mind.

[bolliger32]

Thanks @consideRatio ! We bumped from helm chart v0.9-4300ff5 to v0.10.6 (sorry, kind of a big jump...). Let me know if there are any other details I can provide

[consideRatio]

Ah, this is a jump of JupyterHub from 1.0.0 to 1.2.2, and a jump of OAuthenticator from 0.8.2 to 0.12.1 (old and new dependencies).

I think the issue is clearly stated and defined as it is that we can ignore the past and focus on the now anyhow.