From 547cebe6670303904f131487c4db7c81ae74387f Mon Sep 17 00:00:00 2001 From: Justin Scholz Date: Sat, 26 Aug 2023 20:19:09 +0200 Subject: [PATCH 1/3] Use the state of the login URL to also include the username to be compliant with oauth we still fail with a currently unknown ClientError in finish_login, but at least we successfully get the first token --- github/api/client.py | 6 ++++-- github/client_manager.py | 10 ++++++---- github/commands.py | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/github/api/client.py b/github/api/client.py index 9d5dcae..e771edc 100644 --- a/github/api/client.py +++ b/github/api/client.py @@ -64,8 +64,10 @@ def __init__(self, http: ClientSession, client_id: str, client_secret: str, toke self.token = token self._login_state = "" - def get_login_url(self, redirect_uri: Union[str, URL], scope: str = "user repo") -> URL: - self._login_state = "".join(random.choices(string.ascii_lowercase + string.digits, k=64)) + def get_login_url(self, user_id: str, scope: str = "user repo") -> URL: + user_id = user_id + "%" + self._randomState = "".join(random.choices(string.ascii_lowercase + string.digits, k=64)) + self._login_state = user_id+self._randomState return self.login_url.with_query({ "client_id": self.client_id, "redirect_uri": str(redirect_uri), diff --git a/github/client_manager.py b/github/client_manager.py index 52a0d38..d78ecda 100644 --- a/github/client_manager.py +++ b/github/client_manager.py @@ -92,20 +92,22 @@ async def login_callback(self, request: web.Request) -> web.Response: f"{error_msg}\n\n" f"More info at {error_uri}") try: - user_id = UserID(request.query["user_id"]) + user_id_raw, randomState = (request.query["state"]).split('%') + user_id = UserID(user_id_raw) code = request.query["code"] state = request.query["state"] + except KeyError as e: return web.Response(status=400, text=f"Missing {e.args[0]} parameter") client = self.get(user_id) if not client: - return web.Response(status=401, text="Invalid state token") + return web.Response(status=401, text="Invalid state token because no client") try: await client.finish_login(code, state) except ValueError: - return web.Response(status=401, text="Invalid state token") + return web.Response(status=401, text="Invalid state token because of ValueError") except (KeyError, ClientError): - return web.Response(status=401, text="Failed to finish login") + return web.Response(status=401, text=f"Failed to finish login{ClientError}") resp = await client.query("viewer { login }") user = resp["viewer"]["login"] self.put(user_id, client.token) diff --git a/github/commands.py b/github/commands.py index 7b91e57..4013cfd 100644 --- a/github/commands.py +++ b/github/commands.py @@ -120,8 +120,10 @@ async def login(self, evt: MessageEvent, flags: str, client: Optional[GitHubClie scopes.remove("admin:repo_hook") if "--private" in flags: scopes.append("repo") + # Pass user_id for state in Oauth + user_id = evt.sender login_url = str(self.bot.clients.get(evt.sender, create=True).get_login_url( - redirect_uri=redirect_url, scope=" ".join(scopes))) + user_id=user_id, scope=" ".join(scopes))) if client: username = await client.query("viewer { login }", path="viewer.login") await evt.reply(f"You're already logged in as @{username}, but you can " From 190fdfba51dababa6e52605433e62e490c35432c Mon Sep 17 00:00:00 2001 From: Justin Scholz Date: Sat, 26 Aug 2023 20:34:34 +0200 Subject: [PATCH 2/3] remove no longer needed redirect_url from code --- github/commands.py | 1 - 1 file changed, 1 deletion(-) diff --git a/github/commands.py b/github/commands.py index 4013cfd..49068ae 100644 --- a/github/commands.py +++ b/github/commands.py @@ -111,7 +111,6 @@ async def github(self, evt: MessageEvent) -> None: @command.argument("flags", required=False, pass_raw=True) @authenticated(required=False) async def login(self, evt: MessageEvent, flags: str, client: Optional[GitHubClient]) -> None: - redirect_url = (self.bot.webapp_url / "auth").with_query({"user_id": evt.sender}) flags = flags.lower() scopes = ["user:user", "public_repo", "admin:repo_hook"] if "--no-repo" in flags: From 67dd9f6be724f408e80b6042da610599260378ad Mon Sep 17 00:00:00 2001 From: Justin Scholz Date: Sat, 26 Aug 2023 20:37:36 +0200 Subject: [PATCH 3/3] Cleaned up error output from debugging operations --- github/client_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/client_manager.py b/github/client_manager.py index d78ecda..fbef630 100644 --- a/github/client_manager.py +++ b/github/client_manager.py @@ -107,7 +107,7 @@ async def login_callback(self, request: web.Request) -> web.Response: except ValueError: return web.Response(status=401, text="Invalid state token because of ValueError") except (KeyError, ClientError): - return web.Response(status=401, text=f"Failed to finish login{ClientError}") + return web.Response(status=401, text=f"Failed to finish login") resp = await client.query("viewer { login }") user = resp["viewer"]["login"] self.put(user_id, client.token)