[GitHub] Authorization by GitHub repository access #650
Conversation
Allows for authentication based on repository access, which is required when using outside-collaborators.
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
koendelaat
changed the title
|
@consideRatio any comments? |
| "user" in granted_scopes or "user:email" in granted_scopes | ||
| "user" in granted_scopes | ||
| or "user:email" in granted_scopes | ||
| or not granted_scopes |
There was a problem hiding this comment.
Can you explain this change? If this is called with empty scopes won't it fail to get the user's emails?
There was a problem hiding this comment.
The retrieved scopes will be empty for GitHub Apps https://docs.github.com/en/developers/apps.
So to facilitate this behavior, empty scopes are also allowed.
This is also why we need a change in error handling of the email fetch. This might cause an error, but we don't know upfront what is causing this.
| allowed_repositories = Set( | ||
| config=True, | ||
| help=""" | ||
| Allow users with read access to specified repositories by specifying |
There was a problem hiding this comment.
We also need to think about future maintainability. Could you say a bit about your design decisions to make this conditional on read access as opposed to e.g. write access, making it configurable etc?
There was a problem hiding this comment.
Good point. I thought about it and had the following observations:
- The used app can be either a GitHub OAuth app, or a 'normal' GitHub App
- When using a OAuth app, repo access is completely depending on the users permissions
- When using a GitHub App, repo access is an intersection of the apps permissions and the users permissions
- Read access to a repo is the minimal, and is also relatively easy to verify.
- Write access is harder to verify
Our use-case is an internal (private) repo with example code. We would like to co-create with outside collaborators and for that we give them read-access to this example repository. Also we would give them a JupyterHub environment where they can use these examples. That is why this change is proposed.
|
@manics Any comments on my replies? |
|
@manics, @consideRatio Can you maybe have a look to this PR? |
There was a problem hiding this comment.
I'm positive to this idea in general!
I'm super low on time december, but quick input:
- make who to allow in a repo explicit (only maintainers and up, only triagers and up, etc)
who to allow isnt obvious based on triager/maintainer etc status. I think the config should specify that (repo x + permission y) should have access, and not just repo x and assume a specific permission level is enough for all repos configured.
Allows for GitHub authentication based on repository access, which is required when using outside-collaborators.
Current situation only allows authorization based on org or org/team access. This requires all users to be a member of this org.
With outside-collaborators, we can grant them access to specific repositories, but they don't become member of the organization nor the teams.
This feature allow authorization based on repository access, making JupyterHub accessible to outside-collaborators.