From ebb202f29d5847a6d57387aa064c5004f0ce7b38 Mon Sep 17 00:00:00 2001 From: Trey Moen <50057480+greatgitsby@users.noreply.github.com> Date: Sun, 13 Sep 2026 14:35:46 -0700 Subject: [PATCH] tools: share cancellable browser sign-in (#38893) * tools: share cancellable browser sign-in * tools: invoke auth JSON mode directly from Cabana * tools: require an explicit Python module * tools: order module constants alphabetically * tools: use shared unauthorized handling for devices --- openpilot/tools/lib/auth.py | 101 ++++++++++++++++-------- openpilot/tools/lib/auth_config.py | 3 +- openpilot/tools/replay/py_downloader.cc | 23 ++++-- openpilot/tools/replay/py_downloader.h | 3 + 4 files changed, 88 insertions(+), 42 deletions(-) diff --git a/openpilot/tools/lib/auth.py b/openpilot/tools/lib/auth.py index 6a685e38d9..02df4954a5 100755 --- a/openpilot/tools/lib/auth.py +++ b/openpilot/tools/lib/auth.py @@ -22,34 +22,50 @@ Examples:: """ import argparse +import json import sys +import subprocess import pprint -import webbrowser -from http.server import BaseHTTPRequestHandler, HTTPServer +import time +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import Any -from urllib.parse import parse_qs, urlencode +from urllib.parse import parse_qs, urlencode, urlsplit -from openpilot.tools.lib.api import APIError, CommaApi, UnauthorizedError +from openpilot.tools.lib.api import CommaApi, UnauthorizedError from openpilot.tools.lib.auth_config import set_token, get_token -class ClientRedirectServer(HTTPServer): - query_params: dict[str, Any] = {} +class ClientRedirectServer(ThreadingHTTPServer): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.query_params: dict[str, Any] = {} + self.result_lock = threading.Lock() + + def get_request(self): + request, address = super().get_request() + request.settimeout(1) # Bound incomplete requests, including browser preconnections. + return request, address class ClientRedirectHandler(BaseHTTPRequestHandler): def do_GET(self): - if not self.path.startswith('/auth'): + if urlsplit(self.path).path not in ('/auth', '/auth/'): self.send_response(204) + self.end_headers() return - query = self.path.split('?', 1)[-1] - query_parsed = parse_qs(query, keep_blank_values=True) - self.server.query_params = query_parsed + query_parsed = parse_qs(urlsplit(self.path).query, keep_blank_values=True) + with self.server.result_lock: + if not self.server.query_params and ('code' in query_parsed or 'error' in query_parsed): + self.server.query_params = query_parsed self.send_response(200) self.send_header('Content-type', 'text/plain') self.end_headers() - self.wfile.write(b'Return to the CLI to continue') + try: + self.wfile.write(b'Sign-in received. You can close this tab and return to Cabana or your terminal.') + except ConnectionError: + pass # A closing browser tab must not discard the received callback. def log_message(self, format: str, *args: object) -> None: # noqa: A002 # stdlib override pass # this prevent http server from dumping messages to stdout @@ -94,36 +110,52 @@ def auth_redirect_link(method, port): raise NotImplementedError(f"no redirect implemented for method {method}") -def login(method): - # Let the OS select an available port to avoid colliding with other services. - web_server = ClientRedirectServer(('localhost', 0), ClientRedirectHandler) - oauth_uri = auth_redirect_link(method, web_server.server_port) - print(f'To sign in, use your browser and navigate to {oauth_uri}') - webbrowser.open(oauth_uri, new=2) - - while True: - web_server.handle_request() - if 'code' in web_server.query_params: - break - elif 'error' in web_server.query_params: - print('Authentication Error: "{}". Description: "{}" '.format( - web_server.query_params['error'], - web_server.query_params.get('error_description')), file=sys.stderr) - break - +def login(method, timeout=180): + """Sign in through a browser and save the token, returning a success/error status.""" try: - auth_resp = CommaApi().post('v2/auth/', data={'code': web_server.query_params['code'], 'provider': web_server.query_params['provider']}) - set_token(auth_resp['access_token']) - except APIError as e: - print(f'Authentication Error: {e}', file=sys.stderr) + with ClientRedirectServer(('localhost', 0), ClientRedirectHandler) as server: + url = auth_redirect_link(method, server.server_port) + print(f'To sign in, use your browser and navigate to {url}', file=sys.stderr) + browser = subprocess.Popen(['open' if sys.platform == 'darwin' else 'xdg-open', url], + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + server.timeout = min(0.1, max(0, deadline - time.monotonic())) + server.handle_request() + params = server.query_params + if 'error' in params: + return {"error": "Sign-in was declined. Choose a provider to try again."} + if 'code' in params: + provider = {'google': 'g', 'apple': 'a', 'github': 'h'}[method] + if len(params['code']) != 1 or not params['code'][0].strip() or params.get('provider') != [provider]: + return {"error": "Invalid sign-in response. Please try again."} + response = CommaApi().post('v2/auth/', data={'code': params['code'], 'provider': params['provider']}, timeout=30) + token = response.get('access_token') + if not isinstance(token, str) or not token.strip(): + return {"error": "Sign-in did not return an access token. Please try again."} + CommaApi(token).get('v1/me', timeout=30) + set_token(token) + return {"success": True} + if browser.poll() not in (None, 0): + return {"error": "Could not open your browser. Check your default browser and try again."} + return {"error": "Sign-in timed out. Choose a provider to try again."} + except Exception: + return {"error": "Could not complete sign-in. Check your connection and try again."} if __name__ == '__main__': parser = argparse.ArgumentParser(description='Login to your comma account') parser.add_argument('method', default='google', const='google', nargs='?', choices=['google', 'apple', 'github', 'jwt']) parser.add_argument('jwt', nargs='?') + parser.add_argument('--json', action='store_true', help='Return browser sign-in status as JSON') args = parser.parse_args() + if args.json: + if args.method == 'jwt': + parser.error('--json requires a browser sign-in provider') + print(json.dumps(login(args.method))) + sys.exit(0) + if args.method == 'jwt': if args.jwt is None: print("method JWT selected, but no JWT was provided") @@ -131,7 +163,10 @@ if __name__ == '__main__': set_token(args.jwt) else: - login(args.method) + result = login(args.method) + if "error" in result: + print(result["error"], file=sys.stderr) + sys.exit(1) try: me = CommaApi(token=get_token()).get('/v1/me') diff --git a/openpilot/tools/lib/auth_config.py b/openpilot/tools/lib/auth_config.py index 5966bd34a9..954225d4d3 100644 --- a/openpilot/tools/lib/auth_config.py +++ b/openpilot/tools/lib/auth_config.py @@ -1,6 +1,7 @@ import json import os from openpilot.common.hardware.hw import Paths +from openpilot.common.utils import atomic_write class MissingAuthConfigError(Exception): @@ -18,7 +19,7 @@ def get_token(): def set_token(token): os.makedirs(Paths.config_root(), exist_ok=True) - with open(os.path.join(Paths.config_root(), 'auth.json'), 'w') as f: + with atomic_write(os.path.join(Paths.config_root(), 'auth.json'), overwrite=True) as f: json.dump({'access_token': token}, f) diff --git a/openpilot/tools/replay/py_downloader.cc b/openpilot/tools/replay/py_downloader.cc index a7ab5baa91..286d8a7319 100644 --- a/openpilot/tools/replay/py_downloader.cc +++ b/openpilot/tools/replay/py_downloader.cc @@ -18,6 +18,9 @@ namespace { +constexpr const char *AUTH_MODULE = "openpilot.tools.lib.auth"; +constexpr const char *DOWNLOADER_MODULE = "openpilot.tools.lib.file_downloader"; + static std::mutex handler_mutex; static DownloadProgressHandler progress_handler = nullptr; @@ -30,12 +33,12 @@ void reportProgress(const char *line) { // Run a Python command and capture stdout. Stderr is scanned for PROGRESS lines and otherwise passed // through to the parent's stderr. Returns stdout content. If abort is signaled, kills the child process. -std::string runPython(const std::vector &args, std::atomic *abort = nullptr) { - // Build argv for the downloader module +std::string runPython(const char *module, const std::vector &args, std::atomic *abort = nullptr) { + // Build argv for the Python module std::vector argv; argv.push_back("python3"); argv.push_back("-m"); - argv.push_back("openpilot.tools.lib.file_downloader"); + argv.push_back(module); for (const auto &a : args) { argv.push_back(a.c_str()); } @@ -215,19 +218,23 @@ std::string download(const std::string &url, bool use_cache, std::atomic * if (!use_cache) { args.push_back("--no-cache"); } - return runPython(args, abort); + return runPython(DOWNLOADER_MODULE, args, abort); } std::string decompress(const std::string &path, std::atomic *abort) { - return runPython({"decompress", path}, abort); + return runPython(DOWNLOADER_MODULE, {"decompress", path}, abort); } std::string getRouteFiles(const std::string &route) { - return runPython({"route-files", route}); + return runPython(DOWNLOADER_MODULE, {"route-files", route}); +} + +std::string authenticate(const std::string &provider, std::atomic *abort) { + return runPython(AUTH_MODULE, {provider, "--json"}, abort); } std::string getDevices() { - return runPython({"devices"}); + return runPython(DOWNLOADER_MODULE, {"devices"}); } std::string getDeviceRoutes(const std::string &dongle_id, int64_t start_ms, int64_t end_ms, bool preserved) { @@ -244,7 +251,7 @@ std::string getDeviceRoutes(const std::string &dongle_id, int64_t start_ms, int6 args.push_back(std::to_string(end_ms)); } } - return runPython(args); + return runPython(DOWNLOADER_MODULE, args); } } // namespace PyDownloader diff --git a/openpilot/tools/replay/py_downloader.h b/openpilot/tools/replay/py_downloader.h index 80fab6ab00..fac66dabd5 100644 --- a/openpilot/tools/replay/py_downloader.h +++ b/openpilot/tools/replay/py_downloader.h @@ -18,6 +18,9 @@ std::string decompress(const std::string &path, std::atomic *abort = nullp // Returns JSON string of route files (same format as /v1/route/.../files API) std::string getRouteFiles(const std::string &route); +// Browser sign-in; abort closes the local callback server. Returns a JSON status. +std::string authenticate(const std::string &provider, std::atomic *abort); + // Returns JSON string of user's devices std::string getDevices();