-
Notifications
You must be signed in to change notification settings - Fork 367
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
[GitHub] Authorization by GitHub repository access #650
base: main
Are you sure you want to change the base?
Changes from 3 commits
53ee614
4a8a7c6
3e61aa5
38595f8
404bef0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,6 +114,16 @@ def _userdata_url_default(self): | |
""", | ||
) | ||
|
||
allowed_repositories = Set( | ||
config=True, | ||
help=""" | ||
Allow users with read access to specified repositories by specifying | ||
repositories like `org-a/repo-1`. | ||
|
||
Requires `repo` to be set in `scope`. | ||
""", | ||
) | ||
|
||
populate_teams_in_auth_state = Bool( | ||
False, | ||
config=True, | ||
|
@@ -184,6 +194,17 @@ async def check_allowed(self, username, auth_model): | |
message = f"User {username} is not part of allowed_organizations" | ||
self.log.warning(message) | ||
|
||
if self.allowed_repositories: | ||
access_token = auth_model["auth_state"]["token_response"]["access_token"] | ||
token_type = auth_model["auth_state"]["token_response"]["token_type"] | ||
for repo in self.allowed_repositories: | ||
if await self._check_membership_allowed_repository( | ||
repo, username, access_token, token_type | ||
): | ||
return True | ||
message = f"User {username} does not have access to allowed_repositories" | ||
self.log.warning(message) | ||
|
||
# users should be explicitly allowed via config, otherwise they aren't | ||
return False | ||
|
||
|
@@ -205,23 +226,32 @@ async def update_auth_model(self, auth_model): | |
# - about /user/emails: https://docs.github.com/en/rest/reference/users#list-email-addresses-for-the-authenticated-user | ||
# | ||
# Note that the read:user scope does not imply the user:emails scope! | ||
# Note that the retrieved scopes will be empty for GitHub Apps https://docs.github.com/en/developers/apps | ||
access_token = auth_model["auth_state"]["token_response"]["access_token"] | ||
token_type = auth_model["auth_state"]["token_response"]["token_type"] | ||
granted_scopes = auth_model["auth_state"].get("scope", []) | ||
granted_scopes = [ | ||
scope for scope in auth_model["auth_state"].get("scope", []) if scope | ||
] | ||
if not user_info["email"] and ( | ||
"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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The retrieved scopes will be empty for GitHub Apps https://docs.github.com/en/developers/apps. |
||
): | ||
resp_json = await self.httpfetch( | ||
resp = await self.httpfetch( | ||
f"{self.github_api}/user/emails", | ||
"fetching user emails", | ||
method="GET", | ||
parse_json=False, | ||
raise_error=False, | ||
koendelaat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
headers=self.build_userdata_request_headers(access_token, token_type), | ||
validate_cert=self.validate_server_cert, | ||
) | ||
for val in resp_json: | ||
if val["primary"]: | ||
user_info["email"] = val["email"] | ||
break | ||
if resp.code == 200: | ||
resp_json = json.loads((resp.body or b'').decode('utf8', 'replace')) | ||
for val in resp_json: | ||
if val["primary"]: | ||
user_info["email"] = val["email"] | ||
break | ||
|
||
if self.populate_teams_in_auth_state: | ||
if "read:org" not in self.scope: | ||
|
@@ -332,6 +362,44 @@ async def _check_membership_allowed_organizations( | |
) | ||
return False | ||
|
||
async def _check_membership_allowed_repository( | ||
self, owner_repo, username, access_token, token_type | ||
): | ||
""" | ||
Checks if a user is allowed to read the repo via | ||
GitHub's REST API. The `repo` scope is required to check access. | ||
|
||
The `owner_repo` parameter accepts values like `OWNER/REPO`. | ||
""" | ||
headers = self.build_userdata_request_headers(access_token, token_type) | ||
|
||
# https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#get-a-repository | ||
owner, repo = owner_repo.split("/") | ||
api_url = f"{self.github_api}/repos/{owner}/{repo}" | ||
|
||
self.log.debug(f"Checking GitHub repo access: {username} for {owner}/{repo}?") | ||
resp = await self.httpfetch( | ||
api_url, | ||
parse_json=False, | ||
raise_error=False, | ||
method="GET", | ||
headers=headers, | ||
validate_cert=self.validate_server_cert, | ||
) | ||
if resp.code == 200: | ||
self.log.debug(f"Allowing {username} as access of {owner}/{repo}") | ||
return True | ||
else: | ||
try: | ||
resp_json = json.loads((resp.body or b'').decode('utf8', 'replace')) | ||
message = resp_json.get('message', '') | ||
except ValueError: | ||
message = '' | ||
self.log.debug( | ||
f"{username} does not appear to have access to {owner}/{repo} (status={resp.code}): {message}", | ||
) | ||
return False | ||
|
||
|
||
class LocalGitHubOAuthenticator(LocalAuthenticator, GitHubOAuthenticator): | ||
"""A version that mixes in local system user creation""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I thought about it and had the following observations:
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.