From 578c1f4355c511ce33aaa11eaa061f967458518a Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 12 Mar 2023 20:25:28 +0000 Subject: [PATCH 1/5] Replace fetch(HTTPRequest) with httpfetch(HTTPRequest-args) --- oauthenticator/bitbucket.py | 4 +--- oauthenticator/github.py | 17 +++++++++-------- oauthenticator/gitlab.py | 13 ++++++++----- oauthenticator/globus.py | 9 ++++----- oauthenticator/oauth2.py | 35 ++++++++++++++++++++++++++++------- 5 files changed, 50 insertions(+), 28 deletions(-) diff --git a/oauthenticator/bitbucket.py b/oauthenticator/bitbucket.py index 6f777762..7e3202be 100644 --- a/oauthenticator/bitbucket.py +++ b/oauthenticator/bitbucket.py @@ -2,7 +2,6 @@ Custom Authenticator to use Bitbucket OAuth with JupyterHub """ from jupyterhub.auth import LocalAuthenticator -from tornado.httpclient import HTTPRequest from tornado.httputil import url_concat from traitlets import Set, default @@ -68,8 +67,7 @@ async def _check_membership_allowed_teams(self, username, access_token, token_ty "https://api.bitbucket.org/2.0/workspaces", {'role': 'member'} ) while next_page: - req = HTTPRequest(next_page, method="GET", headers=headers) - resp_json = await self.fetch(req) + resp_json = await self.httpfetch(next_page, method="GET", headers=headers) next_page = resp_json.get('next', None) user_teams = {entry["name"] for entry in resp_json["values"]} diff --git a/oauthenticator/github.py b/oauthenticator/github.py index 1c176b6f..425260fe 100644 --- a/oauthenticator/github.py +++ b/oauthenticator/github.py @@ -7,7 +7,6 @@ from jupyterhub.auth import LocalAuthenticator from requests.utils import parse_header_links -from tornado.httpclient import HTTPRequest from traitlets import Bool, Set, Unicode, default from .oauth2 import OAuthenticator @@ -169,13 +168,13 @@ async def update_auth_model(self, auth_model): if not auth_model["auth_state"]["github_user"]["email"] and ( "user" in granted_scopes or "user:email" in granted_scopes ): - req = HTTPRequest( + resp_json = await self.httpfetch( self.github_api + "/user/emails", + "fetching user emails", method="GET", headers=self.build_userdata_request_headers(access_token, token_type), validate_cert=self.validate_server_cert, ) - resp_json = await self.fetch(req, "fetching user emails") for val in resp_json: if val["primary"]: auth_model["auth_state"]["github_user"]["email"] = val["email"] @@ -210,13 +209,14 @@ async def _paginated_fetch(self, api_url, access_token, token_type): url = api_url content = [] while True: - req = HTTPRequest( + resp = await self.httpfetch( url, + "fetching user teams", + parse_json=False, method="GET", headers=self.build_userdata_request_headers(access_token, token_type), validate_cert=self.validate_server_cert, ) - resp = await self.fetch(req, "fetching user teams", parse_json=False) resp_json = json.loads(resp.body.decode()) content += resp_json @@ -258,14 +258,15 @@ async def _check_membership_allowed_organizations( check_membership_url = self._build_check_membership_url(org, username) - req = HTTPRequest( + self.log.debug(f"Checking GitHub organization membership: {username} in {org}?") + resp = await self.httpfetch( check_membership_url, + parse_json=False, + raise_error=False, method="GET", headers=headers, validate_cert=self.validate_server_cert, ) - self.log.debug(f"Checking GitHub organization membership: {username} in {org}?") - resp = await self.fetch(req, raise_error=False, parse_json=False) if resp.code == 204: self.log.info(f"Allowing {username} as member of {org}") return True diff --git a/oauthenticator/gitlab.py b/oauthenticator/gitlab.py index ac367425..8d81ff9e 100644 --- a/oauthenticator/gitlab.py +++ b/oauthenticator/gitlab.py @@ -160,13 +160,12 @@ async def user_is_authorized(self, auth_model): async def _get_gitlab_version(self, access_token): url = f"{self.gitlab_api}/version" - req = HTTPRequest( + resp_json = await self.httpfetch( url, method="GET", headers=_api_headers(access_token), validate_cert=self.validate_server_cert, ) - resp_json = await self.fetch(req) version_strings = resp_json['version'].split('-')[0].split('.')[:3] version_ints = list(map(int, version_strings)) return version_ints @@ -183,11 +182,15 @@ async def _check_membership_allowed_groups(self, user_id, access_token): ) req = HTTPRequest( url, + ) + resp = await self.httpfetch( + url, + parse_json=False, + raise_error=False, method="GET", headers=headers, validate_cert=self.validate_server_cert, ) - resp = await self.fetch(req, raise_error=False, parse_json=False) if resp.code == 200: return True # user _is_ in group return False @@ -202,13 +205,13 @@ async def _check_membership_allowed_project_ids(self, user_id, access_token): self.member_api_variant, user_id, ) - req = HTTPRequest( + resp_json = await self.httpfetch( url, + raise_error=False, method="GET", headers=headers, validate_cert=self.validate_server_cert, ) - resp_json = await self.fetch(req, raise_error=False) if resp_json: access_level = resp_json.get('access_level', 0) diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py index ede4e953..8f49eedd 100644 --- a/oauthenticator/globus.py +++ b/oauthenticator/globus.py @@ -7,7 +7,6 @@ import urllib from jupyterhub.auth import LocalAuthenticator -from tornado.httpclient import HTTPRequest from tornado.web import HTTPError from traitlets import Bool, List, Set, Unicode, default @@ -247,8 +246,9 @@ async def get_users_groups_ids(self, tokens): # Get list of user's Groups groups_headers = self.get_default_headers() groups_headers['Authorization'] = f'Bearer {groups_token}' - req = HTTPRequest(self.globus_groups_url, method='GET', headers=groups_headers) - groups_resp = await self.fetch(req) + groups_resp = await self.httpfetch( + self.globus_groups_url, method='GET', headers=groups_headers + ) # Build set of Group IDs for group in groups_resp: user_group_ids.add(group['id']) @@ -345,13 +345,12 @@ async def revoke_service_tokens(self, services): all_tokens = [tok for tok in access_tokens + refresh_tokens if tok is not None] for token in all_tokens: - req = HTTPRequest( + await self.httpfetch( self.revocation_url, method="POST", headers=self.get_client_credential_headers(), body=urllib.parse.urlencode({'token': token}), ) - await self.fetch(req) class LocalGlobusOAuthenticator(LocalAuthenticator, GlobusOAuthenticator): diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index b3b0f01f..4c120c9f 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -406,10 +406,11 @@ async def fetch(self, req, label="fetching", parse_json=True, **kwargs): req: tornado HTTPRequest label (str): label describing what is happening, used in log message when the request fails. + parse_json (bool): whether to parse the response as JSON **kwargs: remaining keyword args passed to underlying `client.fetch(req, **kwargs)` Returns: - r: parsed JSON response + parsed JSON response if `parse_json=True`, else `tornado.HTTPResponse` """ try: resp = await self.http_client.fetch(req, **kwargs) @@ -444,6 +445,29 @@ async def fetch(self, req, label="fetching", parse_json=True, **kwargs): else: return resp + async def httpfetch( + self, url, label="fetching", parse_json=True, raise_error=True, **kwargs + ): + """Wrapper for creating and fetching http requests + + logs error responses, parses successful JSON responses + + Args: + url (str): url to fetch + label (str): label describing what is happening, + used in log message when the request fails. + parse_json (bool): whether to parse the response as JSON + raise_error (bool): whether to raise an exception on HTTP errors + **kwargs: remaining keyword args + passed to underlying `tornado.HTTPRequest` + Returns: + parsed JSON response if `parse_json=True`, else `tornado.HTTPResponse` + """ + req = HTTPRequest(url, **kwargs) + return await self.fetch( + req, label=label, parse_json=parse_json, raise_error=raise_error + ) + def login_url(self, base_url): return url_path_join(base_url, "oauth_login") @@ -590,7 +614,7 @@ async def get_token_info(self, handler, params): """ url = url_concat(self.token_url, params) - req = HTTPRequest( + token_info = await self.httpfetch( url, method="POST", headers=self.build_token_info_request_headers(), @@ -598,8 +622,6 @@ async def get_token_info(self, handler, params): validate_cert=self.validate_server_cert, ) - token_info = await self.fetch(req) - if "error_description" in token_info: raise web.HTTPError( 403, @@ -635,15 +657,14 @@ async def token_to_user(self, token_info): if self.userdata_token_method == "url": url = url_concat(url, dict(access_token=access_token)) - req = HTTPRequest( + return await self.httpfetch( url, + "Fetching user info...", method="GET", headers=self.build_userdata_request_headers(access_token, token_type), validate_cert=self.validate_server_cert, ) - return await self.fetch(req, "Fetching user info...") - def build_auth_state_dict(self, token_info, user_info): """ Builds the `auth_state` dict that will be returned by a succesfull `authenticate` method call. From 09f2059bd0eac21dc3a4e506bda50159cef68028 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 12 Mar 2023 20:37:00 +0000 Subject: [PATCH 2/5] Add `http_request_kwargs` to allow extra kwargs to be passed to all tornado.HTTPRequest --- oauthenticator/oauth2.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index 4c120c9f..7fe0e106 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -391,6 +391,20 @@ def _validate_server_cert_default(self): else: return True + http_request_kwargs = Dict( + {}, + help="""Extra default kwargs passed to all HTTPRequests. + + For example, to use a HTTP proxy for all requests: + + c.OAuthenticator.http_request_kwargs = { + "proxy_host": "proxy.example.com", + "proxy_port": 8080, + } + """, + config=True, + ) + http_client = Any() @default("http_client") @@ -450,6 +464,7 @@ async def httpfetch( ): """Wrapper for creating and fetching http requests + Includes http_request_kwargs in request kwargs logs error responses, parses successful JSON responses Args: @@ -459,11 +474,14 @@ async def httpfetch( parse_json (bool): whether to parse the response as JSON raise_error (bool): whether to raise an exception on HTTP errors **kwargs: remaining keyword args - passed to underlying `tornado.HTTPRequest` + passed to underlying `tornado.HTTPRequest`, overrides + `http_request_kwargs` Returns: parsed JSON response if `parse_json=True`, else `tornado.HTTPResponse` """ - req = HTTPRequest(url, **kwargs) + request_kwargs = self.http_request_kwargs.copy() + request_kwargs.update(kwargs) + req = HTTPRequest(url, **request_kwargs) return await self.fetch( req, label=label, parse_json=parse_json, raise_error=raise_error ) From 24fb70b5df9c6b49520eba3b91bc3a9bda8b7d4a Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 12 Mar 2023 21:08:10 +0000 Subject: [PATCH 3/5] Improve MockAsyncHTTPClient.add_host docstring --- oauthenticator/tests/mocks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oauthenticator/tests/mocks.py b/oauthenticator/tests/mocks.py index dcccd0b3..375e052e 100644 --- a/oauthenticator/tests/mocks.py +++ b/oauthenticator/tests/mocks.py @@ -33,7 +33,7 @@ def add_host(self, host, paths): Args: host (str): the host to mock (e.g. 'api.github.com') - paths (list(str|regex, callable)): a list of paths (or regexps for paths) + paths (list[(str|regex, callable)]): a list of paths (or regexps for paths) and callables to be called for those paths. The mock handlers will receive the request as their only argument. @@ -47,7 +47,7 @@ def add_host(self, host, paths): Example:: client.add_host('api.github.com', [ - ('/user': lambda request: {'login': 'name'}) + ('/user', lambda request: {'login': 'name'}) ]) """ self.hosts[host] = paths From f7bb0091e399543d7e75cb7db66c3c087d83f055 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 12 Mar 2023 21:08:53 +0000 Subject: [PATCH 4/5] Mock test OAuthenticator.http_request_kwargs --- oauthenticator/tests/test_oauth2.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/oauthenticator/tests/test_oauth2.py b/oauthenticator/tests/test_oauth2.py index 4b168da0..723b44ec 100644 --- a/oauthenticator/tests/test_oauth2.py +++ b/oauthenticator/tests/test_oauth2.py @@ -1,3 +1,4 @@ +import re import uuid from unittest.mock import Mock @@ -41,3 +42,26 @@ async def test_custom_logout(monkeypatch): await logout_handler.get() assert logout_handler.clear_login_cookie.called logout_handler.clear_cookie.assert_called_once_with(STATE_COOKIE_NAME) + + +async def test_httpfetch(client): + authenticator = OAuthenticator() + authenticator.http_request_kwargs = { + "proxy_host": "proxy.example.org", + "proxy_port": 8080, + } + + # Return request fields as the response so we can examine it + client.add_host( + "example.org", + [ + ( + re.compile(".*"), + lambda req: [req.url, req.method, req.proxy_host, req.proxy_port], + ), + ], + ) + authenticator.http_client = client + + r = await authenticator.httpfetch("http://example.org/a") + assert r == ['http://example.org/a', 'GET', "proxy.example.org", 8080] From d5c9fe8dd0ea10deead500409afc5d1220e5b548 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 12 Mar 2023 22:36:09 +0000 Subject: [PATCH 5/5] Fix (unrelated) linkcheck failure --- docs/source/topic/google.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/topic/google.md b/docs/source/topic/google.md index 1d857f58..fd643086 100644 --- a/docs/source/topic/google.md +++ b/docs/source/topic/google.md @@ -27,7 +27,7 @@ and give it read only access to users and groups. 3. The **Service account permissions (optional)** section that follows is not required. Click **Continue**. 4. On the **Grant users access to this service account** screen, scroll down to the **Create key** section. Click add (`+`) **Create key**. 5. n the side panel that appears, select the format for your key: **JSON** -6. Click **Create**. Your new public/private key pair is generated and downloaded to your machine; it serves as the only copy of this key. For information on how to store it securely, see [Managing service account keys](https://cloud.google.com/iam/docs/understanding-service-accounts#managing_service_accounts). +6. Click **Create**. Your new public/private key pair is generated and downloaded to your machine; it serves as the only copy of this key. For information on how to store it securely, see [Managing service account keys](https://cloud.google.com/iam/docs/understanding-service-accounts). 7. Click **Close** on the **Private key saved to your computer** dialog, then click **Done** to return to the table of your service accounts. 8. Locate the newly-created service account in the table. Under `Actions`, click then **Edit**. 9. In the service account details, click 🔽 **Show domain-wide delegation**, then ensure the **Enable G Suite Domain-wide Delegation** checkbox is checked.