diff --git a/engine/auth/browser_pkce.py b/engine/auth/browser_pkce.py index bc19f67..0bcb079 100644 --- a/engine/auth/browser_pkce.py +++ b/engine/auth/browser_pkce.py @@ -17,6 +17,31 @@ ``is_available()`` is False on a headless server (no browser / no loopback), so the UI falls back to :class:`~engine.auth.paste.PasteTokenProvider`. +NOT WIRED END TO END -- do not enable this provider by setting the Supabase +environment variables alone. Steps 2-4 above each depend on a hosted contract +that ``app.openadapt.ai`` does not implement today: + +* ``GET /login`` reads only ``checkout_session_id`` and ``next`` (clamped to a + same-origin ``/dashboard`` destination). It ignores ``redirect_to``, + ``code_challenge``, ``code_challenge_method`` and ``state``, so the browser + never navigates to the loopback listener and step 3 cannot happen. +* The Supabase ``uri_allow_list`` is rewritten on every production deploy and + contains no loopback entry, so Supabase would reject a ``127.0.0.1`` redirect + even if the page forwarded one. +* ``POST /api/ingest-tokens`` authenticates the browser session cookie and + requires a ``name`` field. It rejects a ``Authorization: Bearer `` with 401, so step 4 cannot mint. + +Because ``_exchange_code`` is only reached AFTER the loopback wait, leaving the +provider enabled makes ``login`` open a browser tab and then block for the full +``timeout`` before failing over to token paste. ``is_available()`` therefore +requires the Supabase configuration up front: the provider reports unavailable +rather than costing the operator a silent multi-minute stall. Restoring it is a +hosted-contract change, not a client change -- see the three bullets above. + +The working, shipped path to a credential is the one-time pairing flow +(:mod:`engine.auth.pairing`), which the hosted control plane does implement. + Reconciliation note: the shared store (:mod:`engine.auth.store`) keys ONE active credential per host and ``auth_header()`` returns exactly one bearer. We therefore fold the Supabase session and the minted ingest token into a single @@ -161,7 +186,17 @@ def _default_open_browser(url: str) -> None: webbrowser.open(url) def is_available(self) -> bool: - """False on a headless box (no browser / no display).""" + """False on a headless box, and False while the flow is unconfigured. + + The Supabase check is deliberately first. ``_exchange_code`` cannot run + without a project URL and anon key, but it is only reached after the + loopback wait -- so an unconfigured provider would open a browser tab + and stall for the whole ``timeout`` before failing over to token paste. + Reporting unavailable up front keeps ``login`` responsive and keeps the + provider chain honest about what it can actually complete. + """ + if not (self._supabase_url and self._supabase_anon_key): + return False if os.environ.get("OPENADAPT_HEADLESS", "").strip(): return False if sys.platform.startswith("linux"): diff --git a/tests/test_engine/test_auth_browser_pkce.py b/tests/test_engine/test_auth_browser_pkce.py index 81165ee..4247505 100644 --- a/tests/test_engine/test_auth_browser_pkce.py +++ b/tests/test_engine/test_auth_browser_pkce.py @@ -49,22 +49,65 @@ def _deliver(): assert receiver.state == "s1" +def _configured(**kwargs) -> BrowserPkceProvider: + """A provider whose Supabase project is configured, as in a wired deployment.""" + kwargs.setdefault("supabase_url", "https://project.supabase.co") + kwargs.setdefault("supabase_anon_key", "anon_key") + return BrowserPkceProvider(**kwargs) + + class TestIsAvailable: def test_headless_env_false(self, monkeypatch) -> None: monkeypatch.setenv("OPENADAPT_HEADLESS", "1") - assert BrowserPkceProvider().is_available() is False + assert _configured().is_available() is False def test_linux_without_display_false(self, monkeypatch) -> None: monkeypatch.delenv("OPENADAPT_HEADLESS", raising=False) monkeypatch.setattr("engine.auth.browser_pkce.sys.platform", "linux") monkeypatch.delenv("DISPLAY", raising=False) monkeypatch.delenv("WAYLAND_DISPLAY", raising=False) + assert _configured().is_available() is False + + def test_macos_true_when_configured(self, monkeypatch) -> None: + monkeypatch.delenv("OPENADAPT_HEADLESS", raising=False) + monkeypatch.setattr("engine.auth.browser_pkce.sys.platform", "darwin") + assert _configured().is_available() is True + + def test_unconfigured_is_unavailable_even_on_a_desktop(self, monkeypatch) -> None: + """An unconfigured provider must not lead the chain and stall the login. + + Without a Supabase project the exchange cannot succeed, but it is only + attempted after the loopback wait -- so reporting available here would + open a browser tab and block for the full timeout before falling back + to token paste. Regression guard for that stall. + """ + monkeypatch.delenv("OPENADAPT_HEADLESS", raising=False) + monkeypatch.delenv("OPENADAPT_SUPABASE_URL", raising=False) + monkeypatch.delenv("OPENADAPT_SUPABASE_ANON_KEY", raising=False) + monkeypatch.setattr("engine.auth.browser_pkce.sys.platform", "darwin") assert BrowserPkceProvider().is_available() is False - def test_macos_true(self, monkeypatch) -> None: + def test_partial_configuration_is_unavailable(self, monkeypatch) -> None: monkeypatch.delenv("OPENADAPT_HEADLESS", raising=False) monkeypatch.setattr("engine.auth.browser_pkce.sys.platform", "darwin") - assert BrowserPkceProvider().is_available() is True + url_only = BrowserPkceProvider( + supabase_url="https://project.supabase.co", supabase_anon_key="" + ) + key_only = BrowserPkceProvider(supabase_url="", supabase_anon_key="anon_key") + assert url_only.is_available() is False + assert key_only.is_available() is False + + def test_unconfigured_provider_never_opens_a_browser(self, monkeypatch) -> None: + """The stall is user-visible as a stray tab; assert none is opened.""" + monkeypatch.delenv("OPENADAPT_HEADLESS", raising=False) + monkeypatch.delenv("OPENADAPT_SUPABASE_URL", raising=False) + monkeypatch.delenv("OPENADAPT_SUPABASE_ANON_KEY", raising=False) + monkeypatch.setattr("engine.auth.browser_pkce.sys.platform", "darwin") + opened: list[str] = [] + provider = BrowserPkceProvider(open_browser=opened.append) + with pytest.raises(RuntimeError, match="unavailable"): + provider.login() + assert opened == [] class TestLogin: @@ -84,7 +127,7 @@ def _deliver(): threading.Thread(target=_deliver, daemon=True).start() - provider = BrowserPkceProvider(host="https://app.openadapt.ai", open_browser=_open_browser) + provider = _configured(host="https://app.openadapt.ai", open_browser=_open_browser) provider._exchange_code = lambda code, verifier, redirect_uri: { # type: ignore[assignment] "access_token": "supabase_access", "refresh_token": "supabase_refresh",