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

@@ -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')

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)
 
 

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<std::string> &args, std::atomic<bool> *abort = nullptr) {
-  // Build argv for the downloader module
+std::string runPython(const char *module, const std::vector<std::string> &args, std::atomic<bool> *abort = nullptr) {
+  // Build argv for the Python module
   std::vector<const char *> 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<bool> *
   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<bool> *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<bool> *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

openpilot/tools/replay/py_downloader.h

@@ -18,6 +18,9 @@ std::string decompress(const std::string &path, std::atomic<bool> *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<bool> *abort);
+
 // Returns JSON string of user's devices
 std::string getDevices();