From 515599677bcc2f6fd4e27cbf5a270f827a904fa0 Mon Sep 17 00:00:00 2001 From: erenes Date: Mon, 5 Apr 2021 16:56:48 +0200 Subject: [PATCH 1/6] Change: Support developer auth flow --- .gitignore | 1 + bananas_cli/authentication.py | 8 +++++--- bananas_cli/cli.py | 5 +++-- bananas_cli/session.py | 2 ++ 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 7a60b85..a5f27c0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.pyc +.env/ diff --git a/bananas_cli/authentication.py b/bananas_cli/authentication.py index 5015e3e..026c767 100644 --- a/bananas_cli/authentication.py +++ b/bananas_cli/authentication.py @@ -26,6 +26,7 @@ class Authenticate: session = None token_filename = None success = False + client_id = None @staticmethod @routes.get("/") @@ -35,7 +36,7 @@ async def callback(request): status, data = await Authenticate.session.post( "/user/token", json={ - "client_id": "ape", + "client_id": Authenticate.client_id, "redirect_uri": "http://localhost:3977/", "code_verifier": Authenticate.code_verifier, "code": code, @@ -78,7 +79,7 @@ async def wait_for_code(): raise Exit -async def authenticate(session, client_id): +async def authenticate(session, client_id, audience): config_folder = click.get_app_dir("bananas-cli") os.makedirs(config_folder, exist_ok=True) token_filename = config_folder + "/token" @@ -95,13 +96,14 @@ async def authenticate(session, client_id): Authenticate.token_filename = token_filename Authenticate.session = session Authenticate.code_verifier = secrets.token_hex(32) + Authenticate.client_id = client_id digest = hashlib.sha256(Authenticate.code_verifier.encode()).digest() code_challenge = base64.urlsafe_b64encode(digest).decode().rstrip("=") status, data = await session.get( "/user/authorize?" - "audience=github&" + f"audience={audience}&" "redirect_uri=http%3A%2F%2Flocalhost%3A3977%2F&" "response_type=code&" f"client_id={client_id}&" diff --git a/bananas_cli/cli.py b/bananas_cli/cli.py index 72c24fc..0dff162 100644 --- a/bananas_cli/cli.py +++ b/bananas_cli/cli.py @@ -18,9 +18,10 @@ ) @click.option("--tus-url", help="BaNaNaS tus URL. (normally the same as --api-url)", metavar="URL") @click.option("--client-id", help="Client-id to use for authentication", default="ape", show_default=True) +@click.option("--audience", help="Audience to use for authentication", default="github", show_default=False) @click.pass_context @task -async def cli(ctx, api_url, tus_url, client_id): +async def cli(ctx, api_url, tus_url, client_id, audience): """ A CLI tool to list, upload, and otherwise modify BaNaNaS content. @@ -43,7 +44,7 @@ async def cli(ctx, api_url, tus_url, client_id): ctx.obj = session await session.start() - await authenticate(session, client_id) + await authenticate(session, client_id, audience) @task diff --git a/bananas_cli/session.py b/bananas_cli/session.py index bf568fa..8fcf525 100644 --- a/bananas_cli/session.py +++ b/bananas_cli/session.py @@ -27,6 +27,8 @@ async def stop(self): async def _read_response(self, response): if response.status in (200, 201, 400, 404): + if response.content_type == "text/html": + return 302, response.url data = await response.json() elif response.status in (301, 302): data = response.headers["Location"] From 513802210b2020f36f9e906b8312e6a742683601 Mon Sep 17 00:00:00 2001 From: erenes Date: Mon, 5 Apr 2021 18:47:23 +0200 Subject: [PATCH 2/6] Fix: do not try to authenticate when requesting help (#4) --- bananas_cli/cli.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bananas_cli/cli.py b/bananas_cli/cli.py index 0dff162..59da5e7 100644 --- a/bananas_cli/cli.py +++ b/bananas_cli/cli.py @@ -28,7 +28,7 @@ async def cli(ctx, api_url, tus_url, client_id, audience): Every option can also be set via an environment variable prefixed with BANANAS_CLI_; for example: - BANANAS_CLI_API_URL="http://localhost:8000" python -m bananas_clie + BANANAS_CLI_API_URL="http://localhost:8000" python -m bananas_cli """ global session @@ -44,6 +44,11 @@ async def cli(ctx, api_url, tus_url, client_id, audience): ctx.obj = session await session.start() + + os_args = click.get_os_args() + if "-h" in os_args or "--help" in os_args: + return + await authenticate(session, client_id, audience) From 4887a9af10b033a46c7be02a3e4294397d3b6cb7 Mon Sep 17 00:00:00 2001 From: erenes Date: Mon, 5 Apr 2021 21:30:07 +0200 Subject: [PATCH 3/6] Fix: Use urllib to combine urls (#5) --- bananas_cli/session.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/bananas_cli/session.py b/bananas_cli/session.py index 8fcf525..9599d45 100644 --- a/bananas_cli/session.py +++ b/bananas_cli/session.py @@ -1,5 +1,6 @@ import aiohttp import logging +import urllib.parse from tusclient.client import TusClient from tusclient.exceptions import TusCommunicationError @@ -38,27 +39,29 @@ async def _read_response(self, response): return response.status, data async def get(self, url): - response = await self.session.get(f"{self.api_url}{url}", headers=self._headers, allow_redirects=False) + full_url = urllib.parse.urljoin(self.api_url, url) + response = await self.session.get(full_url, headers=self._headers, allow_redirects=False) return await self._read_response(response) async def post(self, url, json): - response = await self.session.post( - f"{self.api_url}{url}", json=json, headers=self._headers, allow_redirects=False - ) + full_url = urllib.parse.urljoin(self.api_url, url) + response = await self.session.post(full_url, json=json, headers=self._headers, allow_redirects=False) return await self._read_response(response) async def put(self, url, json): - response = await self.session.put( - f"{self.api_url}{url}", json=json, headers=self._headers, allow_redirects=False - ) + full_url = urllib.parse.urljoin(self.api_url, url) + response = await self.session.put(full_url, json=json, headers=self._headers, allow_redirects=False) return await self._read_response(response) def tus_upload(self, upload_token, fullpath, filename): - tus = TusClient(f"{self.tus_url}/new-package/tus/") + full_url = urllib.parse.urljoin(self.tus_url, "/new-package/tus/") + tus = TusClient(full_url) try: uploader = tus.uploader( - fullpath, chunk_size=UPLOAD_CHUNK_SIZE, metadata={"filename": filename, "upload-token": upload_token} + fullpath, + chunk_size=UPLOAD_CHUNK_SIZE, + metadata={"filename": filename, "upload-token": upload_token}, ) uploader.upload() except TusCommunicationError: From 96fa3011ac094c0406a38c91f27b6987d0f2e842 Mon Sep 17 00:00:00 2001 From: erenes Date: Tue, 6 Apr 2021 21:23:14 +0200 Subject: [PATCH 4/6] Change: Format urls properly for urllib.parse.urljoin --- bananas_cli/authentication.py | 4 ++-- bananas_cli/commands/list_self.py | 2 +- bananas_cli/commands/upload.py | 6 +++--- bananas_cli/session.py | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bananas_cli/authentication.py b/bananas_cli/authentication.py index 026c767..c2b411e 100644 --- a/bananas_cli/authentication.py +++ b/bananas_cli/authentication.py @@ -89,7 +89,7 @@ async def authenticate(session, client_id, audience): bearer_token = f.read() session.set_header("Authorization", f"Bearer {bearer_token}") - status, data = await session.get("/user") + status, data = await session.get("user") if status == 200: return @@ -102,7 +102,7 @@ async def authenticate(session, client_id, audience): code_challenge = base64.urlsafe_b64encode(digest).decode().rstrip("=") status, data = await session.get( - "/user/authorize?" + "user/authorize?" f"audience={audience}&" "redirect_uri=http%3A%2F%2Flocalhost%3A3977%2F&" "response_type=code&" diff --git a/bananas_cli/commands/list_self.py b/bananas_cli/commands/list_self.py index 24edcac..d03502d 100644 --- a/bananas_cli/commands/list_self.py +++ b/bananas_cli/commands/list_self.py @@ -14,7 +14,7 @@ @pass_session @task async def list_self(session): - status, data = await session.get("/package/self") + status, data = await session.get("package/self") if status != 200: log.error(f"Server returned invalid status code {status}: {data}") raise Exit diff --git a/bananas_cli/commands/upload.py b/bananas_cli/commands/upload.py index bfaec2f..ba5c292 100644 --- a/bananas_cli/commands/upload.py +++ b/bananas_cli/commands/upload.py @@ -41,7 +41,7 @@ async def upload(session, version, name, description, url, license, files): starting_part = "/".join(parts) + "/" - status, data = await session.post("/new-package", json={}) + status, data = await session.post("new-package", json={}) if status != 200: log.error(f"Server returned invalid status code {status}: {data}") raise Exit @@ -61,7 +61,7 @@ async def upload(session, version, name, description, url, license, files): if license: payload["license"] = license - status, data = await session.put(f"/new-package/{upload_token}", json=payload) + status, data = await session.put(f"new-package/{upload_token}", json=payload) if status == 400: show_validation_errors(data) raise Exit @@ -74,7 +74,7 @@ async def upload(session, version, name, description, url, license, files): log.info(f"Uploading {filename} ...") session.tus_upload(upload_token, fullpath, filename) - status, data = await session.post(f"/new-package/{upload_token}/publish", json={}) + status, data = await session.post(f"new-package/{upload_token}/publish", json={}) if status == 400: show_validation_errors(data) raise Exit diff --git a/bananas_cli/session.py b/bananas_cli/session.py index 9599d45..f247819 100644 --- a/bananas_cli/session.py +++ b/bananas_cli/session.py @@ -15,8 +15,8 @@ class Session: def __init__(self, api_url, tus_url): self.session = None - self.api_url = api_url - self.tus_url = tus_url + self.api_url = f"${api_url}/" + self.tus_url = f"${tus_url}/" self._headers = {} @@ -54,7 +54,7 @@ async def put(self, url, json): return await self._read_response(response) def tus_upload(self, upload_token, fullpath, filename): - full_url = urllib.parse.urljoin(self.tus_url, "/new-package/tus/") + full_url = urllib.parse.urljoin(self.tus_url, "new-package/tus/") tus = TusClient(full_url) try: From 2a39719e7a30222a315aed3ef812d749e95e8b2a Mon Sep 17 00:00:00 2001 From: erenes Date: Sat, 10 Apr 2021 20:31:53 +0200 Subject: [PATCH 5/6] Fix: String formatting does not work like in C# --- bananas_cli/session.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bananas_cli/session.py b/bananas_cli/session.py index f247819..f9a4ed2 100644 --- a/bananas_cli/session.py +++ b/bananas_cli/session.py @@ -15,8 +15,8 @@ class Session: def __init__(self, api_url, tus_url): self.session = None - self.api_url = f"${api_url}/" - self.tus_url = f"${tus_url}/" + self.api_url = f"{api_url}/" + self.tus_url = f"{tus_url}/" self._headers = {} From 859c8511af318437b8805ccb3bc2d30ba3cbb31e Mon Sep 17 00:00:00 2001 From: erenes Date: Sat, 10 Apr 2021 22:31:46 +0200 Subject: [PATCH 6/6] Fix #3: Show more meaningful errors to cli users --- bananas_cli/cli.py | 16 +++++++++++++--- bananas_cli/commands/upload.py | 3 +++ bananas_cli/session.py | 10 +++++++--- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/bananas_cli/cli.py b/bananas_cli/cli.py index 59da5e7..d66ca41 100644 --- a/bananas_cli/cli.py +++ b/bananas_cli/cli.py @@ -1,9 +1,11 @@ import click import logging +from aiohttp.client_exceptions import ClientConnectorError from .authentication import authenticate from .helpers import task from .session import Session +from .exceptions import Exit log = logging.getLogger(__name__) pass_session = click.make_pass_decorator(Session) @@ -19,9 +21,10 @@ @click.option("--tus-url", help="BaNaNaS tus URL. (normally the same as --api-url)", metavar="URL") @click.option("--client-id", help="Client-id to use for authentication", default="ape", show_default=True) @click.option("--audience", help="Audience to use for authentication", default="github", show_default=False) +@click.option("--verbose", help="Enable verbose output for errors, showing tracebacks", is_flag=True) @click.pass_context @task -async def cli(ctx, api_url, tus_url, client_id, audience): +async def cli(ctx, api_url, tus_url, client_id, audience, verbose): """ A CLI tool to list, upload, and otherwise modify BaNaNaS content. @@ -40,7 +43,7 @@ async def cli(ctx, api_url, tus_url, client_id, audience): if not tus_url: tus_url = api_url - session = Session(api_url, tus_url) + session = Session(api_url, tus_url, verbose) ctx.obj = session await session.start() @@ -49,7 +52,14 @@ async def cli(ctx, api_url, tus_url, client_id, audience): if "-h" in os_args or "--help" in os_args: return - await authenticate(session, client_id, audience) + try: + await authenticate(session, client_id, audience) + except (ClientConnectorError, NameError) as e: + if verbose: + log.exception(e) + else: + log.error(e) + raise Exit @task diff --git a/bananas_cli/commands/upload.py b/bananas_cli/commands/upload.py index ba5c292..95a76a6 100644 --- a/bananas_cli/commands/upload.py +++ b/bananas_cli/commands/upload.py @@ -28,6 +28,9 @@ def show_validation_errors(data): @pass_session @task async def upload(session, version, name, description, url, license, files): + if len(files) == 0: + log.error("No files specified for upload") + return parts = files[0].split("/")[:-1] for filename in files: check_parts = filename.split("/") diff --git a/bananas_cli/session.py b/bananas_cli/session.py index f9a4ed2..10be440 100644 --- a/bananas_cli/session.py +++ b/bananas_cli/session.py @@ -13,12 +13,13 @@ class Session: - def __init__(self, api_url, tus_url): + def __init__(self, api_url, tus_url, verbose): self.session = None self.api_url = f"{api_url}/" self.tus_url = f"{tus_url}/" self._headers = {} + self.verbose = verbose async def start(self): self.session = aiohttp.ClientSession() @@ -64,8 +65,11 @@ def tus_upload(self, upload_token, fullpath, filename): metadata={"filename": filename, "upload-token": upload_token}, ) uploader.upload() - except TusCommunicationError: - log.exception(f"Failed to upload file '{filename}'") + except TusCommunicationError as e: + if self.verbose: + log.exception(f"Failed to upload file '{filename}'") + else: + log.error(f"Failed to upload file '{filename}': {e}") raise Exit def set_header(self, header, value):