diff --git a/CLAUDE.md b/CLAUDE.md index af707b04..c4380ebb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -555,7 +555,10 @@ kbagent config set-folder --project NAME --component-id ID --config-id ID --name kbagent config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] -kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] +kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open] +# oauth-url: opens the link in the default browser (interactive human mode only; never with +# --json, over SSH, in a container or in WSL without wslview); --no-open only prints it. +# Version gate in gotchas.md. kbagent config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID] kbagent config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes] # state-get/state-set (0.84.2+, #593): read/write a config's runtime state via the dedicated diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 8bef3c6a..ffb4422e 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -456,6 +456,7 @@ For detailed response parsing rules and common pitfalls, see [gotchas](reference | Dev branches | [branch-workflow](references/branch-workflow.md) | | **Merge requests** (dev branch -> production with review; conflicts + resolve; auto-merge is destructive; `--json merge` needs an explicit target) | [merge-request-workflow](references/merge-request-workflow.md) | | Encrypting secrets before a config write | [encrypt-workflow](references/encrypt-workflow.md) | +| **Authorizing an OAuth component** (`config oauth-url`; no browser under `--json`, always echo the URL in a fenced block, verify by the config `version` bump) | [oauth-workflow](references/oauth-workflow.md) | | Sync & Git-branching (GitOps) | [sync-workflow](references/sync-workflow.md) | | Sync row-level internals (manifest v3, hoist, encryption) | [sync-rows-workflow](references/sync-rows-workflow.md) | | **Promote configs source -> destination project** (from-scratch GitHub Actions pull -> validate -> push pipeline built on `sync`; PR-gated, cross-project dry-run diff) -- a **separate skill**, not a reference doc | [kbagent-promotion-pipeline](../kbagent-promotion-pipeline/SKILL.md) | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 88075317..738e2788 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -159,7 +159,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. -- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. Open the URL in a browser to complete the OAuth flow. +- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json`, when stdout is not a terminal, or where `auth login` would not open a browser either: SSH session, container, WSL without a working `wslview`); `--no-open` prints it only. The `--json` result has `browser_opened: false`: never tell the user the link is open. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). - `config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID]` -- (since 0.84.2, #593) read a configuration's runtime `state`. Without `--row-id` returns the root config's state; with `--row-id` returns that row's state (a missing row id fails loudly, it does not silently return `{}`). For row-based components the root state node is unused -- read the row state instead. See [config-state-workflow](references/config-state-workflow.md). - `config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes]` -- (since 0.84.2, #593) write a configuration's runtime `state` via the dedicated branch-scoped `PUT .../state` endpoint. `--state` must be a JSON object (not an array/scalar) under 4 MB. `--row-id` targets a row's state instead of the root. `--dry-run` previews the current-vs-new diff (same shape as `config update --dry-run`) without writing; a no-op (new state equals current) short-circuits with `changed: false` and no write. Guarded write: prompts for confirmation unless `--yes` or `--json`. This is the fix for `config update --set 'state...'`, which never reached runtime state (see [gotchas](references/gotchas.md)). See [config-state-workflow](references/config-state-workflow.md) for the seed-before-migrate playbook. diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index c9149ba7..1e519954 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -6057,3 +6057,29 @@ appeared in human mode on a terminal. On older versions `--progress` does not exist and the command fails with a usage error (exit 2) -- check `kbagent version` before passing it. +## An OAuth authorization URL survives only as long as one visual row (SUPPORT-17391) + +The `config oauth-url` link is ~200 chars (`token` + encoded `sapiUrl` + +`#//`), so it fits no terminal row, and two layers then +truncate it. Both end in the same misleading browser message, +`Failed to load config data. Please contact us on support@keboola.com` -- +`token`/`sapiUrl` come first and survive, the config id in the fragment does +not, so the wizard authenticates and then 404s on the configuration: + +- **Rich layout.** `console.print(url)` inserts REAL newlines at the console + width; a copy of one visual row, or a line-based parse of stdout, loses the + tail. `soft_wrap=True` is mandatory for anything printing a URL. +- **Per-row link detection.** Terminals and chat renderers that autodetect URLs + scope detection to one visual row, so a wrapped URL becomes a link to its + first row only. Nothing kbagent prints can prevent that in a renderer it does + not control (e.g. an assistant transcript). + +Since vNEXT the command therefore opens the URL itself (interactive human mode +only; `--no-open` opts out) and prints it unwrapped as a copy fallback. It +skips the open where `auth login` skips it too: an SSH session (`webbrowser` +would start a console browser such as lynx or w3m on the terminal), a +container, and WSL without a working `wslview`. Under +WSL `open_browser` routes through `wslview`, because `webbrowser` keys its +Windows handling off `sys.platform == "win32"` -- `linux` inside WSL -- and +would otherwise find no handler or open a Linux-side browser that shares none +of the logins the flow needs. diff --git a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md new file mode 100644 index 00000000..1c4bf114 --- /dev/null +++ b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md @@ -0,0 +1,30 @@ +# OAuth authorization workflow (`config oauth-url`) + +Getting an OAuth-based component (`keboola.ex-facebook-ads-v2`, `keboola.ex-google-analytics-v4`, ...) authorized for a configuration. + +## Steps + +1. `kbagent config oauth-url --project NAME --component-id ID --config-id ID` + Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The created token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. + Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, never over SSH, in a container or in WSL without a working `wslview`, suppressed by `--no-open`). Under `--json` the result has `browser_opened: false`. + Before you give the link to the user, run `kbagent config detail --project NAME --component-id ID --config-id ID` and note the configuration `version`. Step 3 compares against it. +2. The user completes the provider's consent screen in the browser. +3. Verify: run the same `config detail` again. The authorization is stored when the configuration `version` is higher than the one you noted and `configuration.authorization.oauth_api.id` is set. Do not use `oauth_api.version` as the signal: it is the OAuth Broker API version, the wizard always writes `3`, so it does not change when the user authorizes. Do not claim success without this check; the browser tab tells you nothing. + +## Reporting the link in an answer + +Always give the user the URL. Tell the user that the link is open in the browser only when the result has `browser_opened: true`. Under `--json` it is always `false`: nothing was opened, so ask the user to open the link. Give the URL also when it is `true` -- the user may be on a different machine, or the browser handoff may fail silently (an OS handler accepting a URL is not proof a window appeared). + +Put it in a **fenced code block**, on its own, never as inline prose: + +```` +Open this Facebook Ads authorization link in your browser: + +``` +https://external.keboola.com/oauth/index.html?token=...&sapiUrl=...#/keboola.ex-facebook-ads-v2/01m0zkt36y0mwcvya1apnrvn3k +``` +```` + +The URL is ~200 chars and fits no terminal row. Inline text gets wrapped by the renderer, which then link-detects **only the first visual row** -- the click drops the trailing `#//` and the wizard answers `Failed to load config data. Please contact us on support@keboola.com` even though the token and stack URL are fine. A fenced block is not reflowed, so a copy of it is intact. See [gotchas](gotchas.md). + +Never truncate, shorten or "prettify" the URL, and never split it across lines yourself. diff --git a/src/keboola_agent_cli/auth/environment.py b/src/keboola_agent_cli/auth/environment.py index f670a7d9..6ea51f53 100644 --- a/src/keboola_agent_cli/auth/environment.py +++ b/src/keboola_agent_cli/auth/environment.py @@ -28,6 +28,10 @@ # wslview from stalling the login flow. _WSLVIEW_PROBE_TIMEOUT_SECONDS = 2.0 +# Timeout for handing a URL to `wslview`: it returns as soon as the Windows side +# has accepted the request, so anything longer means the shim is wedged. +_WSLVIEW_OPEN_TIMEOUT_SECONDS = 10.0 + @dataclass(frozen=True) class BrowserEnvironment: @@ -168,6 +172,24 @@ def detect_browser_environment() -> BrowserEnvironment: return BrowserEnvironment(loopback_browser_usable=True, reason="", opener=opener) +def _open_via_wslview(url: str) -> None: + """Thread target for `open_browser` under WSL: hand ``url`` to `wslview`. + + `wslview` passes the URL to the Windows-side default browser. Output is + captured, never logged, and every error is swallowed -- the URL must not + reach stderr (see `_open_silently`). + """ + try: + subprocess.run( + ["wslview", url], + capture_output=True, + timeout=_WSLVIEW_OPEN_TIMEOUT_SECONDS, + check=False, + ) + except Exception: + return + + def open_browser(url: str, *, wait_seconds: float = 0.0) -> bool: """Best-effort `webbrowser.open` on a daemon thread. @@ -182,13 +204,24 @@ def open_browser(url: str, *, wait_seconds: float = 0.0) -> bool: right after the call (``data-app password --open``) passes it, because a daemon thread dies with the process -- possibly before the opener starts. The login flow keeps polling afterwards, so it leaves the default 0. - """ - try: - webbrowser.get() - except webbrowser.Error: - return False - thread = threading.Thread(target=_open_silently, args=(url,), daemon=True) + Under WSL the URL goes to `wslview` first: `webbrowser` keys its Windows + handling off ``sys.platform == "win32"``, which is ``linux`` inside WSL, so + it would either find no handler at all or open a browser in the Linux + session -- a separate profile that shares none of the logins the URL needs. + It runs on the same daemon thread and honors ``wait_seconds`` the same way; + a working `wslview` (probed synchronously) counts as the available handler. + """ + if _env_flag_set("WSL_INTEROP") and _wslview_is_working(): + target = _open_via_wslview + else: + try: + webbrowser.get() + except webbrowser.Error: + return False + target = _open_silently + + thread = threading.Thread(target=target, args=(url,), daemon=True) thread.start() if wait_seconds > 0: thread.join(timeout=wait_seconds) diff --git a/src/keboola_agent_cli/commands/_config_oauth.py b/src/keboola_agent_cli/commands/_config_oauth.py index 8f6016cd..d2d63477 100644 --- a/src/keboola_agent_cli/commands/_config_oauth.py +++ b/src/keboola_agent_cli/commands/_config_oauth.py @@ -16,9 +16,16 @@ import typer from rich.markup import escape +from ..auth.environment import detect_browser_environment, open_browser from ..errors import ConfigError, ErrorCode, KeboolaApiError from ._helpers import get_formatter, get_service, map_error_to_exit_code +# The command exits right after the open, and `open_browser` runs the opener +# on a daemon thread that dies with the process. Same wait as +# `data-app password --open`: long enough for `webbrowser` to start the +# opener, short enough that an opener it blocks on cannot hold the command. +_BROWSER_OPEN_WAIT_SECONDS = 2.0 + def register(app: typer.Typer) -> None: """Mount the oauth-url command onto ``app`` (the ``config`` Typer group).""" @@ -27,7 +34,8 @@ def register(app: typer.Typer) -> None: "oauth-url", rich_help_panel="OAuth", help=( - "Requires master token. Generate an OAuth authorization URL for a component configuration." + "Requires master token. Generate an OAuth authorization URL for a component " + "configuration and open it in the default browser." ), ) def config_oauth_url( @@ -52,16 +60,36 @@ def config_oauth_url( "--redirect-url", help="Optional URL to return to after the OAuth flow completes (sets returnUrl query param)", ), + no_open: bool = typer.Option( + False, + "--no-open", + help="Only print the authorization URL, do not open a browser", + ), ) -> None: """Generate an OAuth authorization URL for a component configuration. - Opens a short-lived, component-scoped authorization link. - The user must open this URL in a browser and grant access. + Creates a short-lived, component-scoped authorization link and opens it in + the default browser, so the URL never has to be clicked or copied: it is + ~200 characters, and a terminal or chat renderer that wraps it turns the + visible link into its first row only, which drops the configuration id + and makes the authorization page answer "Failed to load config data". + The complete URL is still printed for copying elsewhere. + + The browser is left alone in `--json` mode and when stdout is not a + terminal, so scripted and piped callers never get a stray window; + `--no-open` suppresses it in interactive use too. It is also left alone + where `auth login` would not open one either: an SSH session, a + container, or WSL without a working `wslview`. The `--json` output + has `browser_opened: false`: give the URL to the user. \b Examples: kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID + # Print the URL without opening a browser (e.g. authorizing on another machine) + kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID \\ + --no-open + # Redirect back to a custom URL after the OAuth flow completes kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID \\ --redirect-url https://example.com/oauth-done @@ -87,12 +115,37 @@ def config_oauth_url( ) raise typer.Exit(code=map_error_to_exit_code(exc)) from None + url = result["url"] + if formatter.json_mode: + # --json never opens a browser. The key tells an agent so, so that + # it gives the URL to the user and does not report an open. + result["browser_opened"] = False formatter.output(result) - else: + return + + formatter.console.print( + f"[bold]OAuth URL for[/bold] [cyan]{escape(component_id)}[/cyan]/" + f"[cyan]{escape(config_id)}[/cyan]:\n" + ) + # soft_wrap keeps Rich from laying the URL out at the console width: + # by default it inserts real newlines, so copying one visual row yields + # a truncated URL. markup=False stops a literal "[" in the token from + # being read as a style tag. + formatter.console.print(url, soft_wrap=True, highlight=False, markup=False) + + # Same rule as `auth login`: over SSH `webbrowser` would start a console + # browser (lynx, w3m) on the user's terminal, and WSL without `wslview` + # gets a Linux-side browser without the user's logins. The detection + # runs last, so --no-open and piped runs skip its `wslview` probe. + should_open = ( + formatter.console.is_terminal + and not no_open + and detect_browser_environment().loopback_browser_usable + ) + if should_open and open_browser(url, wait_seconds=_BROWSER_OPEN_WAIT_SECONDS): formatter.console.print( - f"[bold]OAuth URL for[/bold] [cyan]{escape(component_id)}[/cyan]/" - f"[cyan]{escape(config_id)}[/cyan]:\n" + "\n[dim]Opened in your default browser. Grant access there.[/dim]" ) - formatter.console.print(f" [link]{result['url']}[/link]") + else: formatter.console.print("\n[dim]Open this URL in a browser and grant access.[/dim]") diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 17fbca62..4f01d022 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -574,9 +574,10 @@ kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] Delete a configuration row. Destructive; --yes to skip confirmation prompt. - kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] + kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open] Return the OAuth authorization URL for a component that uses OAuth authentication. Open the URL in a browser to complete the OAuth flow. + Opens it in the default browser in interactive human mode (never with --json); --no-open only prints it (since vNEXT). kbagent config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID] Read a configuration's runtime state (the same dict --with-state attaches diff --git a/tests/test_auth_environment.py b/tests/test_auth_environment.py index 3d46ec32..db15657e 100644 --- a/tests/test_auth_environment.py +++ b/tests/test_auth_environment.py @@ -10,6 +10,8 @@ from __future__ import annotations import subprocess +import threading +import time from collections.abc import Callable from typing import Any @@ -380,3 +382,134 @@ def _boom(_url: str) -> bool: for thread in list(environment.threading.enumerate()): if thread is not environment.threading.current_thread(): thread.join(timeout=1.0) + + +class TestOpenBrowserUnderWsl: + """WSL is `sys.platform == "linux"`, so `webbrowser` never reaches Windows itself.""" + + @staticmethod + def _join_other_threads() -> None: + for thread in list(environment.threading.enumerate()): + if thread is not environment.threading.current_thread(): + thread.join(timeout=1.0) + + def test_hands_the_url_to_wslview(self, monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[list[str]] = [] + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + calls.append(cmd) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + url = "https://external.keboola.com/oauth/index.html?token=t&sapiUrl=u#/comp/cfg" + + assert open_browser(url) is True + self._join_other_threads() + assert calls == [["wslview", "--version"], ["wslview", url]] + + def test_slow_wslview_does_not_block_a_caller_that_does_not_wait( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`auth login` passes no ``wait_seconds``: the open must stay fire-and-forget.""" + release = threading.Event() + opened = threading.Event() + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + release.wait(timeout=5.0) + opened.set() + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + started = time.monotonic() + assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is True + assert time.monotonic() - started < 1.0 + assert not opened.is_set() + + release.set() + self._join_other_threads() + assert opened.is_set() + + def test_waits_for_wslview_up_to_wait_seconds(self, monkeypatch: pytest.MonkeyPatch) -> None: + """`config oauth-url` exits right after the open, so it waits for `wslview`.""" + opened = threading.Event() + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + time.sleep(0.2) + opened.set() + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=2.0) + is True + ) + assert opened.is_set() + + def test_wait_seconds_bounds_a_wedged_wslview(self, monkeypatch: pytest.MonkeyPatch) -> None: + release = threading.Event() + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + release.wait(timeout=5.0) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + started = time.monotonic() + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=0.2) + is True + ) + assert time.monotonic() - started < 1.0 + + release.set() + self._join_other_threads() + + def test_wslview_error_is_swallowed( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + raise subprocess.TimeoutExpired(cmd, 10.0) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=1.0) + is True + ) + assert "token=t" not in capsys.readouterr().err + + def test_falls_back_to_webbrowser_without_a_working_wslview( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + opened: list[str] = [] + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: None) + monkeypatch.setattr(environment.webbrowser, "get", lambda: object()) + monkeypatch.setattr(environment.webbrowser, "open", opened.append) + + assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is True + + for thread in list(environment.threading.enumerate()): + if thread is not environment.threading.current_thread(): + thread.join(timeout=1.0) + + assert opened == ["https://external.keboola.com/oauth/index.html?token=t"] diff --git a/tests/test_config_row_cli.py b/tests/test_config_row_cli.py index 8af879fa..b5951bf6 100644 --- a/tests/test_config_row_cli.py +++ b/tests/test_config_row_cli.py @@ -1,6 +1,7 @@ """CLI tests for config row-create, row-update, and oauth-url commands.""" import json +import time from pathlib import Path from unittest.mock import MagicMock @@ -8,6 +9,7 @@ from typer.testing import CliRunner, Result from helpers import setup_single_project +from keboola_agent_cli.auth import environment from keboola_agent_cli.cli import app from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.services.config_service import ConfigService @@ -644,6 +646,223 @@ def test_redirect_url_propagates(self, tmp_config_dir: Path) -> None: output = json.loads(result.output) assert output["data"]["redirect_url"] == "https://example.com/done" + @pytest.mark.parametrize("columns", ["40", "80", "120"]) + def test_human_output_keeps_url_on_one_line(self, tmp_config_dir: Path, columns: str) -> None: + """The printed URL carries no inserted newline at any console width.""" + service = self._make_oauth_service(tmp_config_dir) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + env={"COLUMNS": columns, "TERM": "dumb"}, + ) + + assert result.exit_code == 0, result.output + assert OAUTH_RESULT["url"] in result.output + + def _run_oauth_url( + self, + tmp_config_dir: Path, + mp: pytest.MonkeyPatch, + *extra: str, + json_mode: bool = False, + ) -> tuple[Result, list[str]]: + """Run oauth-url with a recording opener; return the result and the opened URLs.""" + service = self._make_oauth_service(tmp_config_dir) + opened: list[str] = [] + + def _record(url: str, **_kwargs: float) -> bool: + opened.append(url) + return True + + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _record) + result = runner.invoke( + app, + [ + *(["--json"] if json_mode else []), + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + *extra, + ], + env={"COLUMNS": "40"}, + ) + return result, opened + + @staticmethod + def _desktop_environment(mp: pytest.MonkeyPatch) -> None: + """A machine where `auth login` would open a browser too.""" + mp.setattr( + "keboola_agent_cli.commands._config_oauth.detect_browser_environment", + lambda: environment.BrowserEnvironment( + loopback_browser_usable=True, reason="", opener="xdg-open" + ), + ) + + def test_interactive_terminal_opens_the_complete_url( + self, tmp_config_dir: Path, force_colour: None + ) -> None: + """On a terminal the whole URL -- not a wrapped fragment -- reaches the browser.""" + with pytest.MonkeyPatch.context() as mp: + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp) + + assert result.exit_code == 0, result.output + assert opened == [OAUTH_RESULT["url"]] + assert "Opened in your default browser" in result.output + + @pytest.mark.parametrize( + ("extra", "json_mode"), + [(["--no-open"], False), ([], True)], + ids=["no-open", "json"], + ) + def test_terminal_run_with_opt_out_leaves_the_browser_alone( + self, tmp_config_dir: Path, force_colour: None, extra: list[str], json_mode: bool + ) -> None: + """--no-open and --json never open a browser, even on a terminal.""" + with pytest.MonkeyPatch.context() as mp: + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp, *extra, json_mode=json_mode) + + assert result.exit_code == 0, result.output + assert opened == [] + assert "Opened in your default browser" not in result.output + + def test_non_terminal_run_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: + """A piped run (CliRunner stdout is no terminal) prints the URL only.""" + with pytest.MonkeyPatch.context() as mp: + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp) + + assert result.exit_code == 0, result.output + assert opened == [] + assert OAUTH_RESULT["url"] in result.output + assert "Open this URL in a browser" in result.output + + @pytest.mark.parametrize( + ("env_var", "value"), + [("SSH_CONNECTION", "10.0.0.1 50000 10.0.0.2 22"), ("WSL_INTEROP", "/run/WSL/1_interop")], + ids=["ssh", "wsl-without-wslview"], + ) + def test_terminal_without_a_usable_browser_leaves_the_browser_alone( + self, tmp_config_dir: Path, force_colour: None, env_var: str, value: str + ) -> None: + """Where `auth login` would not open a browser, oauth-url does not either. + + Runs the real `detect_browser_environment`: over SSH `webbrowser` would + start a console browser on the user's terminal. + """ + with pytest.MonkeyPatch.context() as mp: + for name in ("SSH_CONNECTION", "SSH_TTY", "WSL_INTEROP"): + mp.delenv(name, raising=False) + mp.setattr(environment, "_is_containerized", lambda: False) + mp.setattr(environment, "_wslview_is_working", lambda: False) + mp.setenv(env_var, value) + result, opened = self._run_oauth_url(tmp_config_dir, mp) + + assert result.exit_code == 0, result.output + assert opened == [] + assert "Open this URL in a browser" in result.output + + def test_opener_has_run_when_the_command_returns( + self, tmp_config_dir: Path, force_colour: None + ) -> None: + """The process exits right after the command, and the opener thread dies with it. + + The fake opener is slow on purpose: without the wait, the command + returns (and reports an open) before the opener has run. + """ + service = self._make_oauth_service(tmp_config_dir) + opened: list[str] = [] + + def _slow_open(url: str) -> bool: + time.sleep(0.2) + opened.append(url) + return True + + with pytest.MonkeyPatch.context() as mp: + # The real open_browser, on its webbrowser path: no WSL, no real browser. + mp.delenv("WSL_INTEROP", raising=False) + mp.setattr(environment, "_wslview_is_working", lambda: False) + mp.setattr(environment.webbrowser, "get", lambda: object()) + mp.setattr(environment.webbrowser, "open", _slow_open) + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + self._desktop_environment(mp) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + ) + assert opened == [OAUTH_RESULT["url"]] + + assert result.exit_code == 0, result.output + assert "Opened in your default browser" in result.output + + def test_json_mode_says_no_browser_opened(self, tmp_config_dir: Path) -> None: + """An agent runs with --json: the output must tell it that nothing opened.""" + service = self._make_oauth_service(tmp_config_dir) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + result = _invoke( + tmp_config_dir, + "oauth-url", + [ + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["data"]["browser_opened"] is False + # --------------------------------------------------------------------------- # is_disabled / is_enabled CLI tests