diff --git a/oauthenticator/github.py b/oauthenticator/github.py index df2f0dee..47ba4769 100644 --- a/oauthenticator/github.py +++ b/oauthenticator/github.py @@ -165,11 +165,9 @@ async def check_allowed(self, username, auth_model): ) if user_in_org: return True - else: - # User not found in member list for any organi`ation - self.log.warning( - f"User {username} is not in allowed org list", - ) + self.log.warning( + f"User {username} is not part of allowed_organizations" + ) return False @@ -185,6 +183,8 @@ async def update_auth_model(self, auth_model): Also fetch and store `teams` in auth state if `populate_teams_in_auth_state` is configured. """ + user_info = auth_model["auth_state"][self.user_auth_state_key] + # If a public email is not available, an extra API call has to be made # to a /user/emails using the access token to retrieve emails. The # scopes relevant for this are checked based on this documentation: @@ -194,15 +194,12 @@ async def update_auth_model(self, auth_model): # Note that the read:user scope does not imply the user:emails scope! access_token = auth_model["auth_state"]["token_response"]["access_token"] token_type = auth_model["auth_state"]["token_response"]["token_type"] - granted_scopes = [] - if auth_model["auth_state"]["scope"]: - granted_scopes = auth_model["auth_state"]["scope"] - - if not auth_model["auth_state"]["github_user"]["email"] and ( + granted_scopes = auth_model["auth_state"].get("scope", []) + if not user_info["email"] and ( "user" in granted_scopes or "user:email" in granted_scopes ): resp_json = await self.httpfetch( - self.github_api + "/user/emails", + f"{self.github_api}/user/emails", "fetching user emails", method="GET", headers=self.build_userdata_request_headers(access_token, token_type), @@ -210,7 +207,7 @@ async def update_auth_model(self, auth_model): ) for val in resp_json: if val["primary"]: - auth_model["auth_state"]["github_user"]["email"] = val["email"] + user_info["email"] = val["email"] break if self.populate_teams_in_auth_state: @@ -221,15 +218,10 @@ async def update_auth_model(self, auth_model): "read:org scope is required for populate_teams_in_auth_state functionality to work" ) else: - # Number of teams to request per page - per_page = 100 - - # https://docs.github.com/en/rest/reference/teams#list-teams-for-the-authenticated-user - url = self.github_api + f"/user/teams?per_page={per_page}" - - auth_model["auth_state"]["teams"] = await self._paginated_fetch( - url, access_token, token_type - ) + # https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#list-teams-for-the-authenticated-user + url = f"{self.github_api}/user/teams?per_page=100" + user_teams = await self._paginated_fetch(url, access_token, token_type) + auth_model["auth_state"]["teams"] = user_teams return auth_model @@ -280,23 +272,33 @@ async def _paginated_fetch(self, api_url, access_token, token_type): return content async def _check_membership_allowed_organizations( - self, org, username, access_token, token_type + self, org_team, username, access_token, token_type ): """ Checks if a user is part of an organization or organization's team via GitHub's REST API. The `read:org` scope is required to not only check for public org/team membership. - The `org` parameter accepts values like `org-a` or `org-b:team-1`, + The `org_team` parameter accepts values like `org-a` or `org-b:team-1`, and will adjust to use a the relevant REST API to check either org or team membership. """ headers = self.build_userdata_request_headers(access_token, token_type) - check_membership_url = self._build_check_membership_url(org, username) + + if ":" in org_team: + # check if user is part of an organization's team + # https://docs.github.com/en/rest/teams/members?apiVersion=2022-11-28#get-team-member-legacy + org, team = org_team.split(":") + api_url = f"{self.github_api}/orgs/{org}/teams/{team}/members/{username}" + else: + # check if user is part of an organization + # https://docs.github.com/en/rest/orgs/members?apiVersion=2022-11-28#check-organization-membership-for-a-user + org = org_team + api_url = f"{self.github_api}/orgs/{org}/members/{username}" self.log.debug(f"Checking GitHub organization membership: {username} in {org}?") resp = await self.httpfetch( - check_membership_url, + api_url, parse_json=False, raise_error=False, method="GET", @@ -304,7 +306,7 @@ async def _check_membership_allowed_organizations( validate_cert=self.validate_server_cert, ) if resp.code == 204: - self.log.info(f"Allowing {username} as member of {org}") + self.log.debug(f"Allowing {username} as member of {org_team}") return True else: try: @@ -313,17 +315,10 @@ async def _check_membership_allowed_organizations( except ValueError: message = '' self.log.debug( - f"{username} does not appear to be a member of {org} (status={resp.code}): {message}", + f"{username} does not appear to be a member of {org_team} (status={resp.code}): {message}", ) return False - def _build_check_membership_url(self, org: str, username: str) -> str: - if ":" in org: - org, team = org.split(":") - return f"{self.github_api}/orgs/{org}/teams/{team}/members/{username}" - else: - return f"{self.github_api}/orgs/{org}/members/{username}" - class LocalGitHubOAuthenticator(LocalAuthenticator, GitHubOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/tests/test_github.py b/oauthenticator/tests/test_github.py index 03a591d1..8fd2bbc7 100644 --- a/oauthenticator/tests/test_github.py +++ b/oauthenticator/tests/test_github.py @@ -5,7 +5,7 @@ from io import BytesIO from urllib.parse import parse_qs, urlparse -from pytest import fixture, mark +from pytest import fixture from tornado.httpclient import HTTPResponse from tornado.httputil import HTTPHeaders from traitlets.config import Config @@ -193,23 +193,6 @@ def team_membership(request): client_hosts.pop() -@mark.parametrize( - "org, username, expected", - [ - ("blue", "texas", "https://api.github.com/orgs/blue/members/texas"), - ( - "blue:alpha", - "tucker", - "https://api.github.com/orgs/blue/teams/alpha/members/tucker", - ), - ("red", "grif", "https://api.github.com/orgs/red/members/grif"), - ], -) -async def test_build_check_membership_url(org, username, expected): - output = GitHubOAuthenticator()._build_check_membership_url(org, username) - assert output == expected - - async def test_deprecated_config(caplog): cfg = Config() cfg.GitHubOAuthenticator.github_organization_whitelist = ["jupy"]