From 684dcfd1f584b55285bd19ad761bab238044b268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maty=C3=A1=C5=A1=20Jir=C3=A1t?= Date: Mon, 1 Jun 2026 16:28:12 +0200 Subject: [PATCH 1/2] feat(dev-portal): admin-role PATCH routing + MFA fixes + interactive --password-stdin Three independent fixes against the dev-portal surface that landed in #354, discovered while integrating ABRA Flexi (a real component registration on production apps-api): 1. **Admin-role PATCH routing**. `complexity`, `categories`, `forwardToken`, `forwardTokenDetails`, `injectEnvironment`, `processTimeout`, `requiredMemory`, `features`, and `category` are `.forbidden()` on the apps-api vendor schema (`PATCH /vendors/{vendor}/apps/{app}`) -- but the server's error message is misleading: it says "must be one of: easy, medium, hard" because the enum-validation `.error()` annotation lives on the shared admin schema before `clientAppSchema()` overrides with `.forbidden()`. Source of truth: keboola/developer-portal:src/lib/ validation.js -> clientAppSchema(). Fix: - `DeveloperPortalIdentity.role_hint` becomes a real validator: only `vendor` (default) or `admin` accepted; case-folded; typos raise. The field is now load-bearing, not a free-text label. - `DeveloperPortalClient.patch_app` reads `self._identity.role_hint` and routes admin identities to `PATCH /admin/apps/{app}` (permissive adminAppSchema); vendor identities stay on the vendor endpoint. - `DeveloperPortalService.prepare_patch` preflights: vendor role + admin-only field => fail-fast `VALIDATION_ERROR` with a message that (a) names every offending field, (b) explains why the 422 is misleading, (c) tells the user the exact command to switch identity (`dev-portal identity add --role-hint admin ...`). Admin role bypasses the preflight entirely. - Reads, create, upload-icon, deprecate keep vendor-endpoint behaviour -- only PATCH has a meaningful admin variant on the server. Admin tokens still work on the vendor path for those (superset perms). 2. **MFA login: explicit `challenge` field + actual error surfaced**. User report from a Keboola-org TOTP account: MFA code: 521278 Error: Developer Portal MFA login failed (HTTP 404) Root cause: the apiary spec calls `challenge` optional with default `SOFTWARE_TOKEN_MFA`, but in practice the server 404s when it's omitted. Sending it explicitly fixes it. Single attempt only: an earlier experiment retried with `SMS_MFA` on the same session, but `/auth/login` consumes the session, so the retry always 404'd with "Invalid code or auth state for the user", masking the real first failure (most often a stale 30-second TOTP code from waiting too long to enter it). The error now includes the server response body (truncated to 500 chars) and a hint about TOTP code freshness, so users can tell whether the code was wrong, the session expired, or something else. 3. **`--password-stdin` no longer hangs interactively**. `sys.stdin.read()` waits for EOF, not Enter -- users who pasted a password and pressed Enter sat there until they Ctrl-C'd out. New `_read_password_stdin()` helper branches on `sys.stdin.isatty()`: TTY uses `getpass.getpass()` (hidden, line-based, Enter to confirm); pipe still does `read() -> strip()`. Both `identity add --password-stdin` and `identity edit --password-stdin` route through it. Help text updated to spell out the dual-mode behaviour. Tests (10 new): - TestReadPasswordStdin: TTY -> getpass, pipe -> read. - TestLoginMfaPath::test_mfa_prompt_completes_login: now matches body including `challenge: SOFTWARE_TOKEN_MFA`. - TestLoginMfaPath::test_mfa_failure_surfaces_server_body: real body bubbles up plus stale-TOTP hint. - TestPortalWrites::test_patch_app_vendor_role_hits_vendor_endpoint + test_patch_app_admin_role_hits_admin_endpoint: confirm dispatch. - TestDeveloperPortalIdentity::test_role_hint_accepts_admin + test_role_hint_normalises_case + test_role_hint_rejects_typo. - TestReadsAndPrepareApply::test_prepare_patch_vendor_role_rejects_admin_only_fields + test_prepare_patch_admin_role_allows_admin_only_fields. All 95 dev-portal tests pass; `make check` green (3827 / 8 skipped). --- .claude-plugin/marketplace.json | 2 +- CLAUDE.md | 3 + plugins/kbagent/.claude-plugin/plugin.json | 2 +- plugins/kbagent/agents/keboola-expert.md | 2 +- .../kbagent/references/commands-reference.md | 12 ++- .../kbagent/references/dev-portal-workflow.md | 30 +++++++- .../skills/kbagent/references/gotchas.md | 51 +++++++++++++ pyproject.toml | 2 +- src/keboola_agent_cli/changelog.py | 5 ++ src/keboola_agent_cli/commands/context.py | 22 +++++- src/keboola_agent_cli/commands/dev_portal.py | 58 ++++++++++---- src/keboola_agent_cli/constants.py | 9 +++ src/keboola_agent_cli/dev_portal_client.py | 68 +++++++++++++---- src/keboola_agent_cli/models.py | 45 ++++++++++- .../services/dev_portal_service.py | 42 +++++++++++ tests/test_dev_portal_cli.py | 67 +++++++++++++++++ tests/test_dev_portal_client.py | 71 +++++++++++++++++- tests/test_dev_portal_service.py | 44 +++++++++++ tests/test_e2e.py | 75 +++++++++++++++++++ tests/test_models.py | 36 +++++++++ uv.lock | 2 +- 21 files changed, 603 insertions(+), 45 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 03855a35..870be9b0 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.51.0", + "version": "0.51.1", "source": "./plugins/kbagent", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "category": "development" diff --git a/CLAUDE.md b/CLAUDE.md index a12e932c..db2c8397 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -466,6 +466,9 @@ kbagent dev-portal upload-icon --app VENDOR.APP_ID --file PATH [--identity A] [- kbagent dev-portal publish --app VENDOR.APP_ID [--identity A] [--dry-run] kbagent dev-portal deprecate --app VENDOR.APP_ID [--identity A] [--dry-run] # All writes require an interactive random-code TTY confirm; no --yes / no env bypass. +# Since v0.51.1: --role-hint is validated (vendor/admin) and load-bearing -- admin identities route +# `patch` to PATCH /admin/apps/{app} (permissive schema). Vendor + admin-only field => fail-fast preflight. +# --password-stdin works on TTY (hidden prompt) AND on a pipe (reads to EOF). kbagent encrypt values --project ALIAS --component-id ID --input JSON|@file|- [--output-file PATH] diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index fed78419..aa279f65 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.51.0", + "version": "0.51.1", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "author": { "name": "Keboola", diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 8d1ee75f..c69f8ee0 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -112,7 +112,7 @@ a critical failure. Snowflake `workspace create` `private_key` = 0.47.1+, `sync push` fresh-CREATE variable-link resolution + `--branch ` default-tree promote = 0.47.2+, `feature` group (stack/project/user feature flags, Manage API) = 0.48.0+, - `dev-portal` command group = 0.49.0+, + `dev-portal` = 0.49.0+ (admin-role PATCH = 0.51.1+), headless `__env__` project (`KBAGENT_PROJECT_FROM_ENV=1` + `KBC_TOKEN` + `KBC_STORAGE_API_URL`) + forgiving stack-URL normalization (bare host / full project deep-link) = 0.50.0+, `stream` command group (Data Streams / OTLP) = 0.50.0+, `storage retype` is a future composite), you diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index eebd74a2..c29f37aa 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -214,12 +214,18 @@ Requires the project to be added with its **master ('owner') Storage API token** ## Encryption - `encrypt values --project ALIAS --component-id ID --input JSON|@file|- [--output-file PATH]` -- encrypt #-prefixed secrets via Keboola Encryption API (one-way, no decrypt). Scope: ComponentSecure (project + component). Use for MCP tool call workflows. -## Developer Portal (since v0.49.0) +## Developer Portal (since v0.49.0; admin routing in v0.51.1) Talks to `apps-api.keboola.com`. **Reads are unrestricted; writes always require a human to type a random hex code on a real TTY (no `--yes`, no env bypass, exit 6 on non-TTY).** Use `--dry-run` for the agent-safe preview path. +`--role-hint` is **load-bearing** for `dev-portal patch` (since v0.51.1): `vendor` (default) → `PATCH /vendors/{vendor}/apps/{app}` (restricted schema, the common case); `admin` → `PATCH /admin/apps/{app}` (permissive schema, the only way to set `complexity`, `categories`, `category`, `features`, `forwardToken`, `forwardTokenDetails`, `injectEnvironment`, `processTimeout`, `requiredMemory`). A `vendor` identity with any of those 9 fields in the payload fails fast at preflight with the exact command to switch. + +`--password-stdin` (since v0.51.1) works in both TTY mode (hidden line-based prompt, Enter to confirm) and pipe mode (`echo $PASS | … --password-stdin`, reads to EOF). + +MFA login (since v0.51.1) sends `challenge: SOFTWARE_TOKEN_MFA` explicitly to fix a 404 on personal-account TOTP logins where the apps-api server silently rejects missing-challenge requests despite the spec calling it optional. Single attempt only; failure surfaces the actual server body with a stale-TOTP hint. + ### Identity management -- `dev-portal identity add --alias A --username U [--password P | --password-stdin] [--role-hint vendor|admin] [--vendor V] [--portal-url URL]` -- store a portal login credential per-alias in `config.json` (0600 perms). +- `dev-portal identity add --alias A --username U [--password P | --password-stdin] [--role-hint vendor|admin] [--vendor V] [--portal-url URL]` -- store a portal login credential per-alias in `config.json` (0600 perms). `--role-hint` is validated (`vendor`/`admin`, case-folded) since v0.51.1. - `dev-portal identity list` -- list stored portal identities (no passwords shown). - `dev-portal identity remove --alias A` -- delete an identity alias. - `dev-portal identity edit --alias A [--username U] [--password P|--password-stdin] [--role-hint H] [--vendor V] [--new-alias N]` -- update fields of an identity. @@ -233,7 +239,7 @@ Talks to `apps-api.keboola.com`. **Reads are unrestricted; writes always require ### Write commands (require TTY random-code confirm; use `--dry-run` first) - `dev-portal create --vendor V --data FILE [--identity A] [--dry-run]` -- register a new component from a JSON payload file. -- `dev-portal patch --app VENDOR.APP_ID (--data FILE | --property KEY (--value V | --value-file F)) [--identity A] [--dry-run]` -- update portal properties. `--data` is a full-replace of the provided keys; `--property` targets a single key. +- `dev-portal patch --app VENDOR.APP_ID (--data FILE | --property KEY (--value V | --value-file F)) [--identity A] [--dry-run]` -- update portal properties. Endpoint depends on the identity's `role_hint`: vendor → vendor endpoint, admin → admin endpoint. - `dev-portal upload-icon --app VENDOR.APP_ID --file PATH [--identity A] [--dry-run]` -- upload a PNG/SVG icon. - `dev-portal publish --app VENDOR.APP_ID [--identity A] [--dry-run]` -- publish the component (makes it visible in the UI). - `dev-portal deprecate --app VENDOR.APP_ID [--identity A] [--dry-run]` -- mark the component as deprecated. diff --git a/plugins/kbagent/skills/kbagent/references/dev-portal-workflow.md b/plugins/kbagent/skills/kbagent/references/dev-portal-workflow.md index 24e846e1..79fcb34c 100644 --- a/plugins/kbagent/skills/kbagent/references/dev-portal-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/dev-portal-workflow.md @@ -13,12 +13,38 @@ as KB project tokens, under 0600 protection: ``` kbagent dev-portal identity add --alias vendor-keboola --username service.keboola.xxxxx --password ... --vendor keboola kbagent dev-portal identity add --alias vendor-kds --username service.kds-team.xxxxx --password ... --vendor kds-team -kbagent dev-portal identity add --alias admin-foo --username admin@keboola.com --password-stdin +kbagent dev-portal identity add --alias admin-keboola --username admin@keboola.com --role-hint admin --password-stdin kbagent dev-portal identity use vendor-keboola # default for subsequent commands ``` Service accounts (`service.{vendor}.{id}`) skip MFA. Personal admin -accounts prompt for the MFA code on /dev/tty at login time. +accounts prompt for the MFA code on `/dev/tty` at login time +(`SOFTWARE_TOKEN_MFA`, i.e. a TOTP authenticator app like 1Password / Authy / +Google Authenticator). + +`--password-stdin` works in both pipe mode (`echo $PASS | … --password-stdin`, +reads to EOF) and TTY mode (hidden line-based prompt, Enter to confirm). + +### `role_hint` is load-bearing (since v0.51.1) + +`--role-hint` is **not** a free-text label. It picks which apps-api +endpoint kbagent uses for `dev-portal patch`: + +| Role | PATCH endpoint | Schema | Use for | +|------|----------------|--------|---------| +| `vendor` (default) | `/vendors/{vendor}/apps/{app}` | `clientAppSchema` (restricted) | Cookiecutter-backed properties, schemas, UI options, descriptions, icon | +| `admin` | `/admin/apps/{app}` | `adminAppSchema` (permissive) | The 9 fields forbidden on vendor: `complexity`, `categories`, `category`, `features`, `forwardToken`, `forwardTokenDetails`, `injectEnvironment`, `processTimeout`, `requiredMemory` | + +`role_hint` is validated (`vendor` or `admin`, case-folded). kbagent does +not verify the server-side role of the credential -- if you set `admin` +but the account isn't actually a portal admin, the PATCH fails at the +apps-api with an unambiguous 403. + +When a vendor-role identity tries to patch one of the 9 admin-only +fields, the service **fail-fasts** with a message that names the +offending fields, explains why the server's 422 ("must be one of: ...") +is misleading, and shows the exact command to add and use an admin +identity. No portal call is made. ## Safety contract (read this before issuing any write) diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 9ec440fe..3778f249 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -2331,3 +2331,54 @@ materialize lazily on first import (the bucket/table appear in Storage seconds after the first record arrives, not at create time). `create-source` / `delete` / sink creation are **async**: the API returns a Task that kbagent polls to completion before returning. + +## `dev-portal patch`: admin-only fields need an admin-role identity, vendor PATCH lies about why + +`PATCH /vendors/{vendor}/apps/{app}` on apps-api `.forbidden()`s 9 fields: +`complexity`, `categories`, `category`, `features`, `forwardToken`, +`forwardTokenDetails`, `injectEnvironment`, `processTimeout`, +`requiredMemory`. Sending any of them via a vendor identity returns +`422 Parameter complexity must be one of: easy, medium, hard` (or the +analogous enum message for the other fields). **The message is a server +bug** — the enum-validation `.error()` annotation lives on the shared +admin schema before `clientAppSchema()` overrides with `.forbidden()`, +so when `.forbidden()` fires Joi reuses the unrelated enum message +instead of saying "this field is not allowed here". + +To set any of these you need an admin identity that routes the PATCH +to `PATCH /admin/apps/{app}` instead (since v0.51.1): + +``` +kbagent dev-portal identity add --alias admin-keboola \ + --username admin@keboola.com --role-hint admin --password-stdin +kbagent dev-portal patch --app keboola.ex-foo \ + --data /tmp/patch.json --identity admin-keboola +``` + +With `role_hint: vendor` (the default), kbagent now pre-flights the +payload and fails fast with the same guidance instead of letting the +apps-api return the misleading 422 (since v0.51.1). The 9 forbidden +fields are documented in +[keboola/developer-portal:src/lib/validation.js](https://github.com/keboola/developer-portal/blob/master/src/lib/validation.js) +under `clientAppSchema()`. + +## `dev-portal identity add`: MFA logins for TOTP accounts need the `challenge` field explicit (since v0.51.1) + +The apiary spec calls `challenge` optional with default `SOFTWARE_TOKEN_MFA` +on the second-step `POST /auth/login`, but in practice the server 404s +when it is omitted on a personal-account TOTP login. kbagent now sends +`challenge: SOFTWARE_TOKEN_MFA` explicitly. Single attempt only: +`/auth/login` consumes the session, so any retry with a different +challenge type would always 404 with `Invalid code or auth state for +the user` and mask the real first failure. The raised error includes +the server response body and a hint about TOTP code rotation, so +"stale code" can be distinguished from "wrong code" / "expired session". + +## `dev-portal identity {add,edit} --password-stdin` works in both TTY and pipe mode (since v0.51.1) + +Pre-0.51.1 the flag did `sys.stdin.read().strip()` unconditionally, +which waits for EOF rather than Enter — pasting a password and pressing +Enter just hung until Ctrl-C. The helper now branches on +`sys.stdin.isatty()`: TTY uses `getpass.getpass()` (hidden, line-based, +Enter confirms); pipe (`echo $PASS | kbagent dev-portal identity add +--password-stdin`) still reads to EOF. diff --git a/pyproject.toml b/pyproject.toml index 9301e8be..c6837281 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.51.0" +version = "0.51.1" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index e2090b7f..84ebb708 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,11 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.51.1": [ + "Fix (dev-portal): admin-role PATCH routing. `complexity`, `categories`, `forwardToken`, `forwardTokenDetails`, `injectEnvironment`, `processTimeout`, `requiredMemory`, `features`, and `category` are `.forbidden()` on the apps-api vendor schema (`clientAppSchema` in keboola/developer-portal:src/lib/validation.js) but settable on the admin schema. The vendor PATCH returns a misleading 422 (`Parameter complexity must be one of: easy, medium, hard`) because the enum-validation `.error()` annotation is attached on the shared admin schema before `clientAppSchema()` overrides with `.forbidden()`. `DeveloperPortalIdentity.role_hint` becomes a real validator (`vendor`/`admin`, case-folded, typos raise); `DeveloperPortalClient.patch_app` now reads the role and routes admin identities to `PATCH /admin/apps/{app}` (permissive schema); `DeveloperPortalService.prepare_patch` preflights vendor-role + admin-only-field combinations with a fail-fast error that names every offending field, explains why the 422 is misleading, and tells the user the exact command to switch identity. Admin role bypasses the preflight entirely. Reads, create, upload-icon, deprecate keep vendor-endpoint behaviour -- only PATCH has a meaningful admin variant on the server.", + "Fix (dev-portal): MFA login. The apiary spec calls `challenge` optional with default `SOFTWARE_TOKEN_MFA`, but in practice the server 404s on personal-account TOTP logins when it is omitted -- users saw `Error: Developer Portal MFA login failed (HTTP 404)` with no diagnostic body. The field is now sent explicitly. Single attempt only: an earlier experiment retried with `SMS_MFA` on the same session, but `/auth/login` consumes the session, so the retry always 404'd with `Invalid code or auth state for the user` and masked the real first failure (most often a stale 30-second TOTP code). The raised `KeboolaApiError` now includes the server response body (truncated to 500 chars) plus a hint about TOTP rotation, so users can distinguish wrong-code from stale-code from expired-session.", + "Fix (dev-portal): `--password-stdin` no longer hangs interactively. The old code did `sys.stdin.read().strip()` unconditionally, which waits for EOF (Ctrl-D) rather than for Enter -- users who pasted a password and pressed Enter were stuck until they Ctrl-C'd. The new `_read_password_stdin()` helper branches on `sys.stdin.isatty()`: TTY uses `getpass.getpass()` (hidden, line-based, Enter to confirm); pipe still does `read() -> strip()`. Both `identity add --password-stdin` and `identity edit --password-stdin` route through it. Help text updated to describe the dual-mode behaviour.", + ], "0.51.0": [ "New: Data Streams web UI. The `stream` command group (OTLP/HTTP sources, shipped in 0.50.0) now has a page in the kbagent web UI (`kbagent serve --ui`) under Browse -> Data Streams: list sources, create an OTLP/HTTP source (with sink auto-provisioning + if-not-exists), inspect endpoints/destination with a reveal toggle for the masked OTLP secret, and delete. Full parity with the `kbagent stream *` CLI and the `/stream/*` REST surface.", "Fix: `stream` is now documented in the `kbagent serve` OpenAPI schema. The router was registered and callable, but its tag was missing from `OPENAPI_TAGS`, so `/docs#/stream` rendered as a bare, description-less section outside its logical Data group. A new smoke test asserts every router tag has an OpenAPI description block, so a new router can't ship invisible in `/docs` again.", diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 1f28df5d..05255bea 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -1120,9 +1120,29 @@ **Identity management** -- portal logins are stored per-alias in `config.json`: kbagent dev-portal identity add --alias vendor-keboola \\ - --username service.keboola.xxxxx --password ... --vendor keboola + --username service.keboola.xxxxx --password ... --vendor keboola \\ + --role-hint vendor # default; restricts PATCH to vendor endpoint + kbagent dev-portal identity add --alias admin-keboola \\ + --username admin@keboola.com --role-hint admin --password-stdin kbagent dev-portal identity use vendor-keboola + **`role_hint` is load-bearing (since v0.51.1)**: `vendor` (default) routes + `dev-portal patch` to `PATCH /vendors/{{vendor}}/apps/{{app}}` (restricted + schema); `admin` routes it to `PATCH /admin/apps/{{app}}` (permissive + schema). The admin endpoint is the **only** way to set the 9 fields + apps-api `.forbidden()`s on vendor: `complexity`, `categories`, `category`, + `features`, `forwardToken`, `forwardTokenDetails`, `injectEnvironment`, + `processTimeout`, `requiredMemory`. Sending any of those with a `vendor` + identity fails fast at preflight with the exact command to switch + identity (server-side it would have returned a misleading 422 saying + "must be one of: easy, medium, hard"; that message is a known apps-api + bug -- the field is actually `forbidden()`, not enum-validated). + + **`--password-stdin` (since v0.51.1)** works on TTY (hidden line-based + prompt, Enter to confirm) AND on a pipe (`echo $PASS | … --password-stdin`, + reads to EOF). Pre-0.51.1 the flag hung interactively because it always + waited for EOF. + **Read commands** (unrestricted; good for peer-config research): kbagent --json dev-portal list --vendor keboola diff --git a/src/keboola_agent_cli/commands/dev_portal.py b/src/keboola_agent_cli/commands/dev_portal.py index 7a9e6747..9e25cd17 100644 --- a/src/keboola_agent_cli/commands/dev_portal.py +++ b/src/keboola_agent_cli/commands/dev_portal.py @@ -7,8 +7,11 @@ from __future__ import annotations +import getpass +import sys from typing import TYPE_CHECKING, Any +import click import typer from ..errors import ConfigError, ErrorCode, KeboolaApiError @@ -25,6 +28,16 @@ resolve_identity_alias, ) +# CLI-layer enforcement of the role_hint enum. The Pydantic validator on +# DeveloperPortalIdentity intentionally silent-downgrades unknown values to +# "vendor" for backwards compatibility with pre-0.51.1 config.json files +# that may carry arbitrary free-text strings. That tolerance is wrong at the +# CLI surface, where the user just typed a value RIGHT NOW -- a typo should +# fail loudly, not silently land as "vendor" and confuse the next operation. +# Wiring `click.Choice` here gives the Typer-level rejection (exit 2 + usage +# error) before any model construction. +_ROLE_HINT_CHOICES = ["vendor", "admin"] + dev_portal_app = typer.Typer( help="Keboola Developer Portal — multi-identity, production-safe writes.", no_args_is_help=True, @@ -56,6 +69,19 @@ def _split_app(app: str) -> tuple[str, str]: return vendor, app +def _read_password_stdin() -> str: + """Read a password from stdin. + + TTY -> getpass (hidden, line-based, Enter to confirm). + Pipe/redirected -> read to EOF, strip whitespace. + Using `sys.stdin.read()` unconditionally would hang interactively + until the user sent EOF (Ctrl-D); getpass on TTY fixes that. + """ + if sys.stdin.isatty(): + return getpass.getpass("Password: ").strip() + return sys.stdin.read().strip() + + # ----- Identity subcommands ----- @@ -70,9 +96,14 @@ def identity_add( password_stdin: bool = typer.Option( False, "--password-stdin", - help="Read password from stdin (paste from a secrets manager).", + help="Read password from stdin. On a TTY this is a hidden prompt (Enter to confirm); on a pipe it reads until EOF (e.g. `echo $PASS | … --password-stdin`).", + ), + role_hint: str = typer.Option( + "vendor", + "--role-hint", + click_type=click.Choice(_ROLE_HINT_CHOICES), + help="Identity role: 'vendor' (default) or 'admin'. Routes write commands to different apps-api endpoints -- admin uses PATCH /admin/apps/{app} which accepts complexity/categories/forwardToken/processTimeout/etc. that the vendor endpoint forbids.", ), - role_hint: str = typer.Option("vendor", "--role-hint"), vendor: str | None = typer.Option(None, "--vendor"), portal_url: str = typer.Option( "https://apps-api.keboola.com", @@ -81,9 +112,7 @@ def identity_add( ) -> None: formatter = get_formatter(ctx) if password_stdin: - import sys as _sys - - password = _sys.stdin.read().strip() + password = _read_password_stdin() if not password: raise typer.BadParameter("Pass --password or --password-stdin.") identity = DeveloperPortalIdentity( @@ -149,16 +178,18 @@ def identity_edit( username: str | None = typer.Option(None, "--username"), password: str | None = typer.Option(None, "--password"), password_stdin: bool = typer.Option(False, "--password-stdin"), - role_hint: str | None = typer.Option(None, "--role-hint"), + role_hint: str | None = typer.Option( + None, + "--role-hint", + click_type=click.Choice(_ROLE_HINT_CHOICES), + ), vendor: str | None = typer.Option(None, "--vendor"), new_alias: str | None = typer.Option(None, "--new-alias"), ) -> None: formatter = get_formatter(ctx) svc = get_dev_portal_service(ctx) if password_stdin: - import sys as _sys - - password = _sys.stdin.read().strip() + password = _read_password_stdin() try: if new_alias: svc.rename_identity(alias, new_alias) @@ -255,7 +286,6 @@ def get_app_cmd( # ----- Write commands ----- import json # noqa: E402 -import sys as _sys # noqa: E402 from dataclasses import asdict # noqa: E402 from pathlib import Path # noqa: E402 @@ -270,9 +300,9 @@ def _assert_tty(action_description: str) -> None: and AI agents are rejected before any file or stdin access happens. The full random-code prompt fires later (after the preview) on TTY. """ - is_tty = hasattr(_sys.stdin, "isatty") and _sys.stdin.isatty() + is_tty = hasattr(sys.stdin, "isatty") and sys.stdin.isatty() if not is_tty: - _sys.stderr.write( + sys.stderr.write( f"\nRefusing to {action_description}: this action requires a " "real terminal so a human can type the confirmation code. " "There is no --yes bypass by design.\n" @@ -284,9 +314,7 @@ def _load_payload(data: str | None) -> dict: if data is None: raise typer.BadParameter("--data is required") if data == "-": - import sys as _sys - - return json.loads(_sys.stdin.read()) + return json.loads(sys.stdin.read()) return json.loads(Path(data).read_text()) diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 166d6715..f6c8e725 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -27,6 +27,15 @@ # --- API Error Handling --- MAX_API_ERROR_LENGTH: int = 500 +# --- Developer Portal MFA --- +# Challenge type sent on the second `/auth/login` step (after the first call +# returns a `session` token). The apiary spec documents `SOFTWARE_TOKEN_MFA` +# (TOTP authenticator app) as the default and `SMS_MFA` as the only other +# member, but in practice the server 404s when the field is omitted, so we +# send it explicitly. If apps-api ever adds a third member (e.g. EMAIL_OTP), +# wire it in here and add a per-identity `mfa_challenge` field. +DP_MFA_CHALLENGE_TYPE: str = "SOFTWARE_TOKEN_MFA" + # --- UNEXPECTED_ERROR truncation --- # Unhandled ``Exception`` messages surfaced to per-project error envelopes are # truncated to this many characters before being returned. Exceptions can diff --git a/src/keboola_agent_cli/dev_portal_client.py b/src/keboola_agent_cli/dev_portal_client.py index d11800e0..753d0759 100644 --- a/src/keboola_agent_cli/dev_portal_client.py +++ b/src/keboola_agent_cli/dev_portal_client.py @@ -21,6 +21,7 @@ import httpx +from .constants import DP_MFA_CHALLENGE_TYPE, MAX_API_ERROR_LENGTH from .errors import ErrorCode, KeboolaApiError from .http_base import BaseHttpClient from .models import DeveloperPortalIdentity @@ -119,6 +120,17 @@ def _login(self, username: str, password: str) -> str: ) def _login_with_mfa(self, username: str, session: str) -> str: + """Confirm an MFA-gated login. + + Per the Keboola Developer Portal apiary spec, the same POST /auth/login + endpoint accepts {email, session, code, challenge}. The `challenge` + field is documented as optional with default SOFTWARE_TOKEN_MFA, but + in practice the server rejects calls that omit it (404 with the + misleading "must be one of" enum message attached to the admin schema). + Send it explicitly. Single attempt only -- /auth/login consumes the + session, so any retry on the same session always 404s with "Invalid + code or auth state for the user" regardless of the new challenge type. + """ code = _tty_prompt("MFA code: ") if not code: raise KeboolaApiError( @@ -130,28 +142,44 @@ def _login_with_mfa(self, username: str, session: str) -> str: ), error_code=ErrorCode.DP_MFA_REQUIRED, ) + body = { + "email": username, + "session": session, + "code": code.strip(), + "challenge": DP_MFA_CHALLENGE_TYPE, + } try: - resp = self._client.post( - "/auth/login", - json={"email": username, "session": session, "code": code.strip()}, - ) + resp = self._client.post("/auth/login", json=body) except httpx.HTTPError as exc: raise KeboolaApiError( message=f"Developer Portal MFA login transport error: {exc}", error_code=ErrorCode.CONNECTION_ERROR, ) from exc - if resp.status_code != 200: - raise KeboolaApiError( - message=f"Developer Portal MFA login failed (HTTP {resp.status_code})", - error_code=ErrorCode.DP_LOGIN_FAILED, - ) - payload = resp.json() - if not isinstance(payload, dict) or not payload.get("token"): + if resp.status_code == 200: + payload = resp.json() + if isinstance(payload, dict) and payload.get("token"): + return payload["token"] raise KeboolaApiError( - message="Developer Portal MFA login response missing token", + message=( + "Developer Portal MFA login returned HTTP 200 but no " + f"'token' field in response: {payload!r}" + ), error_code=ErrorCode.DP_LOGIN_FAILED, ) - return payload["token"] + try: + body_text = resp.text[:MAX_API_ERROR_LENGTH] + except (UnicodeDecodeError, AttributeError): + body_text = "" + raise KeboolaApiError( + message=( + f"Developer Portal MFA login failed (HTTP {resp.status_code}): " + f"{body_text}. If your TOTP code rotates every 30s, this is " + "often a stale code -- retry promptly. If the server says " + "'Invalid code or auth state' on a fresh session, the code " + "itself was wrong." + ), + error_code=ErrorCode.DP_LOGIN_FAILED, + ) # ----- Reads ----- @@ -190,8 +218,20 @@ def create_app(self, vendor: str, payload: dict[str, Any]) -> dict[str, Any]: return resp.json() def patch_app(self, vendor: str, app_id: str, payload: dict[str, Any]) -> dict[str, Any]: + """PATCH an app. Routes by identity role: + - admin -> PATCH /admin/apps/{app_id} (permissive schema, accepts the + 9 fields forbidden() on the vendor schema: complexity, categories, + forwardToken, forwardTokenDetails, injectEnvironment, processTimeout, + requiredMemory, features, category). + - vendor -> PATCH /vendors/{vendor}/apps/{app_id} (default, restricted + schema). The `vendor` arg is still required for the path. + """ self._ensure_authenticated() - resp = self._do_request("PATCH", f"/vendors/{vendor}/apps/{app_id}", json=payload) + if self._identity.role_hint == "admin": + path = f"/admin/apps/{app_id}" + else: + path = f"/vendors/{vendor}/apps/{app_id}" + resp = self._do_request("PATCH", path, json=payload) if resp.status_code not in (200, 204): self._raise_dp_error(resp, action="patch app", vendor=vendor, app_id=app_id) return resp.json() if resp.content else {} diff --git a/src/keboola_agent_cli/models.py b/src/keboola_agent_cli/models.py index 51353e24..79679669 100644 --- a/src/keboola_agent_cli/models.py +++ b/src/keboola_agent_cli/models.py @@ -1,5 +1,6 @@ """Pydantic models shared across all layers of the application.""" +import sys from typing import Any from urllib.parse import urlparse @@ -104,8 +105,15 @@ class DeveloperPortalIdentity(BaseModel): role_hint: str = Field( default="vendor", description=( - "Free-text label shown in `dev-portal identity list` " - "(e.g. 'vendor', 'admin'). Not validated against the portal." + "Identity role: 'vendor' (default) or 'admin'. Load-bearing -- " + "write commands route to different apps-api endpoints based on " + "role: 'admin' uses PATCH /admin/apps/{app} (permissive schema, " + "can set complexity/categories/forwardToken/processTimeout/etc.); " + "'vendor' uses PATCH /vendors/{vendor}/apps/{app} (those fields " + "are forbidden()). kbagent does not verify the server-side role " + "of the credential -- if you set 'admin' but the account isn't " + "actually a portal admin, the write fails at the apps-api with " + "an unambiguous 403." ), ) vendor: str | None = Field( @@ -129,6 +137,39 @@ def validate_portal_url(cls, v: str) -> str: raise ValueError(f"Portal URL must use https:// scheme, got: {v!r}") return v + @field_validator("role_hint", mode="before") + @classmethod + def validate_role_hint(cls, v: object) -> str: + """Normalise `role_hint` to the validated enum {vendor, admin}. + + Before v0.51.1 the field was free-text and documented as "not + validated against the portal", so existing on-disk configs may + carry arbitrary strings (e.g. 'keboola-admin', empty string, + non-string types from hand-edits). A strict raise would crash + `ConfigStore.load()` -> the entire CLI on startup for every + pre-0.51.1 user with a non-standard value; that's a worse UX + than a silent downgrade. + + Behaviour: + - "vendor" / "admin" (case-insensitive, whitespace-stripped) + pass through normalised. + - Anything else is downgraded to "vendor" with a one-shot stderr + warning. The user still sees what happened; the CLI keeps + working. To force admin routing they can rerun + `dev-portal identity edit --alias A --role-hint admin`. + """ + if not isinstance(v, str): + v = "" if v is None else str(v) + normalized = v.strip().lower() + if normalized in ("vendor", "admin"): + return normalized + sys.stderr.write( + f"Warning: role_hint={v!r} is not 'vendor' or 'admin' -- " + "downgrading to 'vendor'. Use `kbagent dev-portal identity " + "edit --alias --role-hint admin` to switch.\n" + ) + return "vendor" + class PermissionPolicy(BaseModel): """Firewall-style permission policy for CLI and MCP operations. diff --git a/src/keboola_agent_cli/services/dev_portal_service.py b/src/keboola_agent_cli/services/dev_portal_service.py index 5dfd2893..39e7306f 100644 --- a/src/keboola_agent_cli/services/dev_portal_service.py +++ b/src/keboola_agent_cli/services/dev_portal_service.py @@ -35,6 +35,27 @@ "documentationUrl", ) +# Fields that the apps-api server `.forbidden()`s on the vendor PATCH endpoint +# (PATCH /vendors/{vendor}/apps/{app}). Settable only via PATCH /admin/apps/{app} +# with an admin-role token. Sending any of these on the vendor endpoint returns +# a 422 with a misleading "must be one of: ..." error message because the +# enum-validation error annotation is attached in the shared admin schema +# before clientAppSchema overrides with `.forbidden()`. Source of truth: +# keboola/developer-portal:src/lib/validation.js -> clientAppSchema(). +_ADMIN_ONLY_PATCH_FIELDS = frozenset( + { + "category", + "categories", + "complexity", + "features", + "forwardToken", + "forwardTokenDetails", + "injectEnvironment", + "processTimeout", + "requiredMemory", + } +) + @dataclass(frozen=True) class FieldDiff: @@ -214,6 +235,27 @@ def prepare_patch( app_id: str, payload: dict[str, Any], ) -> PendingPatch: + ident = self._resolve_identity(alias) + admin_only_in_payload = sorted(_ADMIN_ONLY_PATCH_FIELDS & set(payload.keys())) + if admin_only_in_payload and ident.role_hint != "admin": + raise KeboolaApiError( + message=( + f"Cannot patch admin-only field(s) via the vendor endpoint: " + f"{admin_only_in_payload}. The apps-api server forbids these " + f"on PATCH /vendors/{vendor}/apps/{app_id} (the 422 you'd " + "otherwise see -- 'must be one of: ...' -- is a misleading " + "server-side message hiding the real reason: the field is " + "forbidden() on the vendor schema, not enum-validated). " + "Switch to an admin identity to route this PATCH through " + "/admin/apps/{app} instead: `kbagent dev-portal identity add " + "--alias --username --role-hint admin " + "--password-stdin` and re-run with `--identity `. " + "Alternatively, drop the field and ask a Developer Portal " + "admin to set it. Canonical list: keboola/developer-portal " + "src/lib/validation.js -> clientAppSchema()." + ), + error_code=ErrorCode.VALIDATION_ERROR, + ) with self._authed_client(alias) as client: current = client.get_app(vendor, app_id) diff = [ diff --git a/tests/test_dev_portal_cli.py b/tests/test_dev_portal_cli.py index 0874d8de..ffc2ea44 100644 --- a/tests/test_dev_portal_cli.py +++ b/tests/test_dev_portal_cli.py @@ -12,6 +12,73 @@ runner = CliRunner() +class TestReadPasswordStdin: + """--password-stdin must work in BOTH TTY mode (hidden getpass prompt, + Enter to confirm) AND pipe mode (read until EOF). The original version + called sys.stdin.read() unconditionally, which hung interactively until + the user sent Ctrl-D.""" + + def test_tty_uses_getpass(self, monkeypatch): + from keboola_agent_cli.commands.dev_portal import _read_password_stdin + + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + monkeypatch.setattr("getpass.getpass", lambda prompt="": "pw-typed\n") + assert _read_password_stdin() == "pw-typed" + + def test_pipe_reads_until_eof(self, monkeypatch): + import io + import sys as _sys + + from keboola_agent_cli.commands.dev_portal import _read_password_stdin + + fake_stdin = io.StringIO("pw-piped\n") + # Use monkeypatch.setattr (not direct attribute assignment) -- ty rejects + # `fake_stdin.isatty = lambda: False` because the slot expects `(self) -> bool` + # and the lambda's signature is `() -> Literal[False]`. monkeypatch handles + # the duck-typed override cleanly without a ty: ignore. + monkeypatch.setattr(fake_stdin, "isatty", lambda: False) + monkeypatch.setattr(_sys, "stdin", fake_stdin) + assert _read_password_stdin() == "pw-piped" + + def test_identity_add_password_stdin_end_to_end(self, tmp_config_dir): + """End-to-end CliRunner test: --password-stdin in pipe mode (the + CliRunner's stdin is not a TTY) must thread the piped password through + Typer's flag parsing into the helper and into the persisted identity. + Catches a regression where the flag and the helper get rewired + independently and the password silently lands as empty.""" + from keboola_agent_cli.config_store import ConfigStore + + with patch( + "keboola_agent_cli.services.dev_portal_service.DeveloperPortalService.add_identity" + ) as add_: + r = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "--json", + "dev-portal", + "identity", + "add", + "--alias", + "piped", + "--username", + "u", + "--password-stdin", + ], + input="my-piped-secret\n", + ) + assert r.exit_code == 0, r.output + add_.assert_called_once() + # The identity object passed to the service must carry the piped password, + # stripped of trailing newline. + call_args = add_.call_args + identity = call_args.args[1] if len(call_args.args) > 1 else call_args.kwargs["identity"] + assert identity.password == "my-piped-secret" + # Sanity: nothing was persisted on disk (add_identity is mocked). + assert ConfigStore(tmp_config_dir, source="cli-flag").load().dev_portal_identities == {} + + class TestIdentityCommands: def test_identity_add_and_list_json(self, tmp_config_dir): with patch( diff --git a/tests/test_dev_portal_client.py b/tests/test_dev_portal_client.py index dc1abac5..b37cfef5 100644 --- a/tests/test_dev_portal_client.py +++ b/tests/test_dev_portal_client.py @@ -43,6 +43,13 @@ def test_login_bad_credentials_raises(self, httpx_mock): class TestLoginMfaPath: def test_mfa_prompt_completes_login(self, httpx_mock, monkeypatch): + """TOTP authenticator app path: explicit SOFTWARE_TOKEN_MFA challenge. + + The server requires the `challenge` field even though the apiary spec + calls it optional with a SOFTWARE_TOKEN_MFA default -- omitting it + gives a 404 with the misleading "must be one of: ..." enum error + attached to the admin schema. Send it explicitly to avoid that. + """ httpx_mock.add_response( method="POST", url="https://apps-api.keboola.com/auth/login", @@ -55,9 +62,13 @@ def test_mfa_prompt_completes_login(self, httpx_mock, monkeypatch): url="https://apps-api.keboola.com/auth/login", json={"token": "Bearer xyz"}, status_code=200, - match_json={"email": "u@k.com", "session": "sess-1", "code": "123456"}, + match_json={ + "email": "u@k.com", + "session": "sess-1", + "code": "123456", + "challenge": "SOFTWARE_TOKEN_MFA", + }, ) - # Mock the /dev/tty MFA prompt. monkeypatch.setattr( "keboola_agent_cli.dev_portal_client._tty_prompt", lambda label, secret=False: "123456", @@ -67,6 +78,41 @@ def test_mfa_prompt_completes_login(self, httpx_mock, monkeypatch): client._ensure_authenticated() assert client._bearer == "Bearer xyz" + def test_mfa_failure_surfaces_server_body(self, httpx_mock, monkeypatch): + """Single attempt only. Surface the actual server body so the user can + tell whether the code was wrong, the session expired, or something else. + Hint about stale TOTP appears in the message.""" + httpx_mock.add_response( + method="POST", + url="https://apps-api.keboola.com/auth/login", + json={"session": "sess-1"}, + status_code=200, + match_json={"email": "u@k.com", "password": "p"}, + ) + httpx_mock.add_response( + method="POST", + url="https://apps-api.keboola.com/auth/login", + status_code=400, + text='{"errorMessage":"Invalid code","errorCode":400}', + match_json={ + "email": "u@k.com", + "session": "sess-1", + "code": "999999", + "challenge": "SOFTWARE_TOKEN_MFA", + }, + ) + monkeypatch.setattr( + "keboola_agent_cli.dev_portal_client._tty_prompt", + lambda label, secret=False: "999999", + ) + ident = DeveloperPortalIdentity(username="u@k.com", password="p") + with DeveloperPortalClient(ident) as client: + with pytest.raises(KeboolaApiError) as exc: + client._ensure_authenticated() + assert exc.value.error_code == ErrorCode.DP_LOGIN_FAILED + assert "Invalid code" in str(exc.value) + assert "TOTP" in str(exc.value) or "stale" in str(exc.value) + def test_mfa_no_tty_raises_mfa_required(self, httpx_mock, monkeypatch): httpx_mock.add_response( method="POST", @@ -138,7 +184,7 @@ def test_create_app(self, httpx_mock): ) assert resp["id"] == "ex-foo" - def test_patch_app(self, httpx_mock): + def test_patch_app_vendor_role_hits_vendor_endpoint(self, httpx_mock): httpx_mock.add_response( method="POST", url="https://apps-api.keboola.com/auth/login", @@ -153,6 +199,25 @@ def test_patch_app(self, httpx_mock): resp = client.patch_app("keboola", "keboola.ex-foo", {"name": "Foo 2"}) assert resp["name"] == "Foo 2" + def test_patch_app_admin_role_hits_admin_endpoint(self, httpx_mock): + """An admin-role identity must route PATCH to /admin/apps/{app} so the + permissive schema accepts admin-only fields like complexity. httpx_mock + has NO entry for /vendors/.../apps/... -- if the client wrongly routes + there, the test fails with an unmocked-request error.""" + httpx_mock.add_response( + method="POST", + url="https://apps-api.keboola.com/auth/login", + json={"token": "Bearer admin-bearer"}, + ) + httpx_mock.add_response( + method="PATCH", + url="https://apps-api.keboola.com/admin/apps/keboola.ex-foo", + json={"id": "ex-foo", "complexity": "easy"}, + ) + with DeveloperPortalClient(_identity(role_hint="admin")) as client: + resp = client.patch_app("keboola", "keboola.ex-foo", {"complexity": "easy"}) + assert resp["complexity"] == "easy" + def test_publish_app(self, httpx_mock): httpx_mock.add_response( method="POST", diff --git a/tests/test_dev_portal_service.py b/tests/test_dev_portal_service.py index 5099b877..f1e87e77 100644 --- a/tests/test_dev_portal_service.py +++ b/tests/test_dev_portal_service.py @@ -126,6 +126,50 @@ def test_apply_patch_calls_client(self, service, fake_client): assert result["name"] == "New" fake_client.patch_app.assert_called_with("keboola", "keboola.ex-a", {"name": "New"}) + def test_prepare_patch_vendor_role_rejects_admin_only_fields( + self, service, fake_client, config_store + ): + """Vendor-role identity + admin-only field => fail-fast with switch-to-admin guidance. + + No portal call happens (we fail before get_app). The error names every + offending field and tells the user how to switch identity. + """ + from keboola_agent_cli.models import DeveloperPortalIdentity + + fake_client._ensure_authenticated.return_value = None + ident = DeveloperPortalIdentity(username="u", password="p", role_hint="vendor") + service.add_identity("vendor-alpha", ident) + with pytest.raises(KeboolaApiError) as exc: + service.prepare_patch( + "vendor-alpha", + "keboola", + "keboola.ex-a", + {"name": "New", "complexity": "easy", "categories": ["x"]}, + ) + assert exc.value.error_code == ErrorCode.VALIDATION_ERROR + assert "complexity" in str(exc.value) + assert "categories" in str(exc.value) + assert "admin" in str(exc.value).lower() + fake_client.get_app.assert_not_called() + + def test_prepare_patch_admin_role_allows_admin_only_fields( + self, service, fake_client, config_store + ): + """Admin-role identity bypasses the preflight; the client routes the + actual PATCH to /admin/apps (verified in client tests). Here we just + check the service's preflight gate is permissive for admin role.""" + from keboola_agent_cli.models import DeveloperPortalIdentity + + fake_client._ensure_authenticated.return_value = None + ident = DeveloperPortalIdentity(username="a", password="p", role_hint="admin") + service.add_identity("admin-bob", ident) + fake_client.get_app.return_value = {"id": "ex-a", "complexity": None} + pending = service.prepare_patch( + "admin-bob", "keboola", "keboola.ex-a", {"complexity": "easy"} + ) + keys = {d.key for d in pending.diff} + assert keys == {"complexity"} + def test_prepare_publish_missing_fields(self, service, fake_client): self._setup(service, fake_client) fake_client.get_app.return_value = { diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 23573ae7..4d67737d 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -10696,6 +10696,81 @@ def test_list_apps_against_real_portal(self) -> None: ) assert result.exit_code == 0, result.output + def test_role_hint_typo_rejected_at_cli_layer(self) -> None: + """`identity add --role-hint vendr` is rejected by Typer's + `click.Choice(["vendor", "admin"])` validator before any model + construction happens (since v0.51.1). + + Offline -- the rejection is a Typer usage error (exit 2), no + network is touched. Note the layering: the Pydantic validator on + the model itself deliberately *downgrades* unknown values to + "vendor" with a stderr warning, so legacy free-text values in a + pre-0.51.1 `config.json` still load. That tolerance is appropriate + for `ConfigStore.load()` but wrong at the CLI -- a typo the user + just typed should fail loudly, not silently land as "vendor". The + `click.Choice` wiring in `commands/dev_portal.py` provides that + separation. + """ + result = _invoke( + self.config_dir, + [ + "dev-portal", + "identity", + "add", + "--alias", + "bad-role", + "--username", + "u", + "--password", + "p", + "--role-hint", + "vendr", # typo + ], + ) + # Typer/Click usage error -> exit 2 + assert result.exit_code == 2 + assert "vendor" in result.output.lower() or "admin" in result.output.lower() + + def test_vendor_role_admin_only_field_fails_fast(self) -> None: + """`prepare_patch` preflight refuses admin-only fields on a vendor identity + (since v0.51.1). Runs offline -- preflight fires before any portal call, + so no creds needed. Verifies the user-facing error names the offending + field and points at the admin-identity workaround. + """ + from keboola_agent_cli.config_store import ConfigStore + from keboola_agent_cli.models import DeveloperPortalIdentity + + store = ConfigStore(self.config_dir, source="cli-flag") + store.add_dev_portal_identity( + "vendor-e2e", + DeveloperPortalIdentity( + username="u", password="p", vendor="keboola", role_hint="vendor" + ), + ) + + payload_path = self.config_dir / "patch.json" + payload_path.write_text(json.dumps({"complexity": "easy"})) + result = _invoke( + self.config_dir, + [ + "dev-portal", + "patch", + "--app", + "keboola.ex-bogus", + "--data", + str(payload_path), + "--identity", + "vendor-e2e", + "--dry-run", + ], + ) + # exit non-zero because validation error + assert result.exit_code != 0 + # error mentions the offending field and the admin workaround + out_lower = result.output.lower() + assert "complexity" in out_lower + assert "admin" in out_lower + @skip_without_credentials @pytest.mark.e2e diff --git a/tests/test_models.py b/tests/test_models.py index de49817b..20cf4404 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -583,6 +583,42 @@ def test_accepts_staging_https_portal_url(self) -> None: ) assert ident.portal_url == "https://apps-api.staging.keboola.dev" + def test_role_hint_accepts_admin(self) -> None: + from keboola_agent_cli.models import DeveloperPortalIdentity + + ident = DeveloperPortalIdentity(username="u", password="p", role_hint="admin") + assert ident.role_hint == "admin" + + def test_role_hint_normalises_case(self) -> None: + from keboola_agent_cli.models import DeveloperPortalIdentity + + ident = DeveloperPortalIdentity(username="u", password="p", role_hint="ADMIN") + assert ident.role_hint == "admin" + + def test_role_hint_typo_downgrades_to_vendor_with_warning(self, capsys) -> None: + """Typos do NOT raise: pre-0.51.1 configs had free-text values, so we + normalise unknown strings to 'vendor' with a stderr warning to keep + ConfigStore.load() from crashing the CLI on upgrade.""" + from keboola_agent_cli.models import DeveloperPortalIdentity + + ident = DeveloperPortalIdentity(username="u", password="p", role_hint="vendr") + assert ident.role_hint == "vendor" + captured = capsys.readouterr() + assert "role_hint" in captured.err + assert "downgrading" in captured.err + + def test_legacy_freetext_role_hint_loads_cleanly(self, capsys) -> None: + """Backwards compat: a config.json carrying any free-text role_hint + (allowed pre-0.51.1) must round-trip through Pydantic without raising. + Empty strings, hand-edited values, even non-string types get normalised.""" + from keboola_agent_cli.models import DeveloperPortalIdentity + + for legacy in ("keboola-admin", "", " ADMIN ", 42): + ident = DeveloperPortalIdentity.model_validate( + {"username": "u", "password": "p", "role_hint": legacy} + ) + assert ident.role_hint in ("vendor", "admin") + class TestAppConfigDevPortalFields: """Tests for AppConfig dev_portal_identities and default_dev_portal_identity fields.""" diff --git a/uv.lock b/uv.lock index 5afa2343..3bff4baf 100644 --- a/uv.lock +++ b/uv.lock @@ -496,7 +496,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.51.0" +version = "0.51.1" source = { editable = "." } dependencies = [ { name = "croniter" }, From 20710dbd820546425585644e46f277a34c055281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maty=C3=A1=C5=A1=20Jir=C3=A1t?= Date: Mon, 1 Jun 2026 17:54:02 +0200 Subject: [PATCH 2/2] test(dev-portal): use DP_MFA_CHALLENGE_TYPE constant in client tests Replace the two hardcoded "SOFTWARE_TOKEN_MFA" match_json literals with the DP_MFA_CHALLENGE_TYPE constant from constants.py, following through on the NIT-1 constant extraction so the tests can't silently diverge from the client. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_dev_portal_client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_dev_portal_client.py b/tests/test_dev_portal_client.py index b37cfef5..d47ab880 100644 --- a/tests/test_dev_portal_client.py +++ b/tests/test_dev_portal_client.py @@ -4,6 +4,7 @@ import pytest +from keboola_agent_cli.constants import DP_MFA_CHALLENGE_TYPE from keboola_agent_cli.dev_portal_client import DeveloperPortalClient from keboola_agent_cli.errors import ErrorCode, KeboolaApiError from keboola_agent_cli.models import DeveloperPortalIdentity @@ -66,7 +67,7 @@ def test_mfa_prompt_completes_login(self, httpx_mock, monkeypatch): "email": "u@k.com", "session": "sess-1", "code": "123456", - "challenge": "SOFTWARE_TOKEN_MFA", + "challenge": DP_MFA_CHALLENGE_TYPE, }, ) monkeypatch.setattr( @@ -98,7 +99,7 @@ def test_mfa_failure_surfaces_server_body(self, httpx_mock, monkeypatch): "email": "u@k.com", "session": "sess-1", "code": "999999", - "challenge": "SOFTWARE_TOKEN_MFA", + "challenge": DP_MFA_CHALLENGE_TYPE, }, ) monkeypatch.setattr(