Skip to content

Filter environments by user access#2940

Merged
marcelovilla merged 8 commits intonebari-dev:mainfrom
soapy1:get-conda-envs-filter-by-user
Mar 10, 2025
Merged

Filter environments by user access#2940
marcelovilla merged 8 commits intonebari-dev:mainfrom
soapy1:get-conda-envs-filter-by-user

Conversation

@soapy1
Copy link
Copy Markdown
Contributor

@soapy1 soapy1 commented Feb 6, 2025

This PR updates the get_conda_environments function called by jhub apps so that only the environments the user has access to are shown. This is achieved by generating a token for permissions that match the jupyterhub users' conda-store permissions.

Reference Issues or PRs

#2193

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

How to test this PR?

  1. To test this PR you must have
    a. an installation of Nebari with jhub apps enabled
    b. at least 2 users with some conda environments
  2. Log in as the first user
  3. Navigate to the nebari home screen
  4. Click on the deploy app button
  5. Click on the drop down to select the conda store environment for the app
  6. Notice that all the environments the user has access to are available - that is default environments + its own environments, but not environments from the second user
  7. Repeat for the second user

if groups is not None:
for group in groups:
group = group.replace("/", "")
role_bindings["role_bindings"][f"{group}/*"] = ["viewer"]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a pretty big assumption here, that the keycloak groups map directly to the conda-store namespaces. Is that true for nebari?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no direct way to create a new namespace through the ui, and group names do match namespaces, though there is also a namespace per user that matches their user name.

That being said, it is possible to create a new namespace and give permissions to use it with an arbitrary name using the conda-store api.

Additionally, some groups, like admins, have access to more namespaces than just their own.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a doc (or piece of existing code) that describes the relationship between keycloak permissions/groups/etc and conda-store privileges? I want to be able to ensure the user is able to see all the namespaces+environments it has access to.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is custom auth logic at

which does the mapping

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmmm, I see that's not quite what I'm looking for. The user_info dict provided ultimately comes from the jupyter api /users endpoint https://jupyterhub.readthedocs.io/en/5.2.1/reference/rest-api.html#operation/get-user which provides the users roles and groups. Here is an example of the output

{
  "name": "string",
  "admin": true,
  "roles": [
    "string"
  ],
  "groups": [
    "string"
  ],
  "server": "string",
  "pending": "spawn",
  "last_activity": "2019-08-24T14:15:22Z",
  "servers": {
  },
  "auth_state": {}
}

Is there some piece of code or docs that describe the relationship between hub users and keycloak users or conda store permissions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@viniciusdc do you have any insight here for @soapy1?

Copy link
Copy Markdown
Member

@krassowski krassowski Feb 11, 2025

Choose a reason for hiding this comment

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

relationship between hub users and keycloak users

Chiming in as I worked on this - both groups and roles are taken from Keycloak since:

@viniciusdc can confirm if anything changed since as I was not reviewing PRs that followed, but I suspect this is still the case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @krassowski those links were very helpful in piecing together a model of what is going on here. This part of the authenticator is also notable https://github.com/nebari-dev/nebari/blob/main/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/conda-store/config/conda_store_config.py#L415.

I think this implementation where the permissions are constructed based on the group provided by the Jupyter user object should be correct. I added a few extra comments to explain the motivation.

This was pretty convoluted to piece together. I think it would be good if at least one other person also went thru the exercise of following the permissions flow from keycloak thru Jupyter and conda-store to double check that this approach is valid.

It would be nice if there was some kind of document that outlined the intended design of how permissions work in nebari. Maybe as part of the tasks in this issue #2304?

@soapy1 soapy1 force-pushed the get-conda-envs-filter-by-user branch 2 times, most recently from f5f3382 to 8fd1157 Compare February 7, 2025 05:08
@dcmcand dcmcand requested review from a team, dcmcand and marcelovilla and removed request for a team February 7, 2025 10:55
@soapy1 soapy1 force-pushed the get-conda-envs-filter-by-user branch from a8a4d83 to aee3dff Compare February 12, 2025 17:24
@soapy1 soapy1 requested a review from a team as a code owner February 12, 2025 17:24
@soapy1 soapy1 force-pushed the get-conda-envs-filter-by-user branch from f4d13ba to 9e71468 Compare February 12, 2025 18:29
@marcelovilla marcelovilla added this to the 2025.3.1 release milestone Mar 4, 2025
Copy link
Copy Markdown
Contributor

@dcmcand dcmcand left a comment

Choose a reason for hiding this comment

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

@soapy1 this works great on a new deploy, but doesn't work if you have an existing deploy. If you have an existing deploy and upgrade you can still see all of the envs in jhub-apps.

@github-project-automation github-project-automation bot moved this from New 🚦 to Changes requested 🧱 in 🪴 Nebari Project Management Mar 5, 2025
@soapy1
Copy link
Copy Markdown
Contributor Author

soapy1 commented Mar 5, 2025

Thanks @dcmcand for testing this out!

but doesn't work if you have an existing deploy. If you have an existing deploy and upgrade you can still see all of the envs in jhub-apps.

I think I'm going to need a bit more information in order to debug this. In particular:

  • what version of nebari did you start with?
  • how did you upgrade?
  • is this behaviour true for all users, or for a set of users with some characteristic?
  • what are the keycloak groups that the offending user is part of? are they an admin?
  • what version of keycloak is running?

@dcmcand dcmcand self-assigned this Mar 6, 2025
Copy link
Copy Markdown
Contributor

@dcmcand dcmcand left a comment

Choose a reason for hiding this comment

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

🚀

@dcmcand dcmcand added the status: approved 💪🏾 This PR has been reviewed and approved for merge label Mar 7, 2025
dcmcand

This comment was marked as duplicate.

@marcelovilla
Copy link
Copy Markdown
Member

@dcmcand tested this again and did not run into the issue that was mentioned before. Thus, I'm merging this.

Thanks @soapy1 🚀 !

@marcelovilla marcelovilla merged commit 81c1f4f into nebari-dev:main Mar 10, 2025
13 of 14 checks passed
@github-project-automation github-project-automation bot moved this from Changes requested 🧱 to Done 💪🏾 in 🪴 Nebari Project Management Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: approved 💪🏾 This PR has been reviewed and approved for merge

Projects

Status: Done 💪🏾

Development

Successfully merging this pull request may close these issues.

4 participants