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

Remove feature for restricting profile options based on github org membership #4022

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 12 additions & 36 deletions docs/hub-deployment-guide/configure-auth/github-orgs.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are a couple more places where this file needs changes like:

  • the title, as it mentions organization based access (or keep the tile, but add a warning right below it as it is not longer supported)
  • point 4 that says **Edit the non-secret config under config/clusters/<cluster_name>/<hub_name>.values.yaml.** has a scopes example specific to orgnanizations (line 72 to line 90)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch @GeorgianaElena! I cleaned that out and removed a bunch of options there as well, as we have nowhere we use read:user at all - it's read:org everywhere. Let's just standardize to that.

Original file line number Diff line number Diff line change
Expand Up @@ -66,30 +66,9 @@ You can remove yourself from the org once you have confirmed that login is worki
...
```

4. **Edit the non-secret config under `config/clusters/<cluster_name>/<hub_name>.values.yaml`.**
You should make sure the matching hub config takes one of the following forms.

To authenticate against a GitHub organisation (Note the `read:user` scope. See comment box below.):

```yaml
jupyterhub:
custom:
2i2c:
add_staff_user_ids_to_admin_users: true
add_staff_user_ids_of_type: github
hub:
config:
JupyterHub:
authenticator_class: github
GitHubOAuthenticator:
oauth_callback_url: https://{{ HUB_DOMAIN }}/hub/oauth_callback
allowed_organizations:
- ORG_NAME
scope:
- read:user
```

To authenticate against a GitHub Team (Note the `read:org` scope. See the comment box below.):
4. **Edit the non-secret config under `config/clusters/<cluster_name>/<hub_name>.values.yaml`,**
making sure we ask for enough permissions (`read:org`) so we know what organizations (or
teams) users are a part of

```yaml
jupyterhub:
Expand All @@ -105,19 +84,11 @@ You can remove yourself from the org once you have confirmed that login is worki
oauth_callback_url: https://{{ HUB_DOMAIN }}/hub/oauth_callback
allowed_organizations:
- ORG_NAME:TEAM_NAME
- ORG_NAME
scope:
- read:org
```

```{admonition} A note on scopes
When authenticating against a whole organisation, we used the `read:user` scope in the example above.
This means that the GitHub OAuth App will read the _user's_ profile to determine whether the currently authenticating user is a member of the listed organisation. **It also requires the user to have their membership of the organisation publicly listed otherwise authentication will fail, even if they are valid members.**

To avoid this requirement, you may choose to use the `read:org` scope instead. This grants the GitHub OAuth App permission to read the profile of the _whole organisation_, however, and may be more powerful than the organisation owners wish to grant. So use your best judgment here.

When authenticating against a GitHub Team, we are required to use the `read:org` scope as the GitHub OAuth App needs to know which teams belong to the organisation as well as the members of the specified team.
```

````{note}
Allowing access to a specific GitHub team, let's say `ORG_NAME:TEAM_NAME`, doesn't mean that the users that are only members of the TEAM_NAME sub-teams, e.g. `ORG_NAME:TEAM_NAME:SUB_TEAM_NAME`, will get access too.

Expand Down Expand Up @@ -173,8 +144,7 @@ This only works if the hub is already set to allow people only from certain GitH
to log in.

The key `allowed_teams` can be set for any profile definition, with a list of GitHub
teams (formatted as `<github-org>:<team-name>`) or GitHub organizations (formatted
just as `<github-org>`) that will get access to that profile. Users
teams (formatted as `<github-org>:<team-name>`) that will get access to that profile. Users
need to be a member of any one of the listed teams for access. The list of teams a user
is part of is fetched at login time - so if the user is added to a GitHub team, they need
to log out and log back in to the JupyterHub (not necessarily to GitHub!) to see the new
Expand All @@ -200,7 +170,7 @@ To enable this access,

If `populate_teams_in_auth_state` is not set, this entire feature is disabled.

2. Specify which teams or orgs should have access to which profiles with an
2. Specify which teams should have access to which profiles with an
`allowed_teams` key under `profileList`:

```yaml
Expand Down Expand Up @@ -231,6 +201,12 @@ To enable this access,
`allowed_teams` so 2i2c engineers can log in to debug issues. If
`allowed_teams` is not set, that profile is not available to anyone.

```{note}
We used to allow restricting which profiles users can see based on what
org they were a part of, rather than just the *teams* they were a part of.
We no longer support this.
```

### Enabling team based access on hub with pre-existing users

If this is being enabled for users on a hub with *pre-existing* users, they
Expand Down
23 changes: 1 addition & 22 deletions helm-charts/basehub/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1043,36 +1043,15 @@ jupyterhub:
allowed_profiles.append(profile)
continue

# allowed_teams can be "org" or "org:team", and we check
# membership just in time for orgs if needed
# casefold them so we can do case insensitive comparisons, as github itself is case insensitive (but preserving)
# casefold teams so we can do case insensitive comparisons, as github itself is case insensitive (but preserving)
# for orgs and teams
allowed_orgs = set([o.casefold() for o in allowed_teams if ':' not in o])
allowed_teams = set([t.casefold() for t in allowed_teams if ':' in t])

if allowed_teams & teams:
print(f"Allowing profile {profile['display_name']} for user {spawner.user.name} based on team membership")
allowed_profiles.append(profile)
continue

if "token_response" in auth_state:
access_token = auth_state["token_response"]["access_token"]
token_type = auth_state["token_response"]["token_type"]
else:
# token_response was introduced to auth_state in
# oauthenticator 16, so this is adjusting to an auth_state
# set by oauthenticator 15
access_token = auth_state["access_token"]
token_type = "token"
for allowed_org in allowed_orgs:
user_in_allowed_org = await spawner.authenticator._check_membership_allowed_organizations(
allowed_org, spawner.user.name, access_token, token_type
)
if user_in_allowed_org:
print(f"Allowing profile {profile['display_name']} for user {spawner.user.name} based on org membership")
allowed_profiles.append(profile)
break

if len(allowed_profiles) == 0:
# If no profiles are allowed, user should not be able to spawn anything!
# If we don't explicitly stop this, user will be logged into the 'default' settings
Expand Down