From 207535ebe54c4efa76c9471fe4f4bc5af80b267a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 10 Aug 2026 11:19:53 +0200 Subject: [PATCH 01/13] feat(auth): add kbagent auth login-password for unattended CI login Password-grant login (email + password + optional stdlib-computed TOTP code) as the deliberate, CI-safe exception to the existing browser-only PKCE/device-code login policy -- for CI/CD service accounts that use email+password (non-SAML) auth and cannot open a browser. Co-Authored-By: Claude Sonnet 5 --- .claude-plugin/marketplace.json | 2 +- CLAUDE.md | 17 +- docs/auth.md | 73 +++++- docs/error-codes.md | 1 + plugins/kbagent/.claude-plugin/plugin.json | 2 +- plugins/kbagent/skills/kbagent/SKILL.md | 1 + .../kbagent/references/commands-reference.md | 9 +- pyproject.toml | 2 +- src/keboola_agent_cli/auth/auth_client.py | 30 +++ src/keboola_agent_cli/auth/models.py | 18 ++ src/keboola_agent_cli/auth/totp.py | 26 ++ src/keboola_agent_cli/changelog.py | 17 ++ src/keboola_agent_cli/commands/auth.py | 89 ++++++- src/keboola_agent_cli/commands/context.py | 34 ++- src/keboola_agent_cli/constants.py | 10 + src/keboola_agent_cli/errors.py | 4 + src/keboola_agent_cli/permissions.py | 1 + .../services/auth_service.py | 231 ++++++++++++------ tests/test_auth_client.py | 104 ++++++++ tests/test_auth_service.py | 77 ++++++ tests/test_auth_totp.py | 45 ++++ tests/test_cli_auth.py | 95 +++++++ uv.lock | 2 +- 23 files changed, 782 insertions(+), 108 deletions(-) create mode 100644 src/keboola_agent_cli/auth/totp.py create mode 100644 tests/test_auth_totp.py diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index b7fbedc1..8b14b579 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.83.0", + "version": "0.84.0", "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 810707fb..2fd23178 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -290,13 +290,28 @@ plugins/kbagent/ # Headless / token-only (0.50.0+): export KBAGENT_PROJECT_FROM_ENV=1 + KBC_TOKEN + KBC_STORAGE_API_URL to synthesize an in-memory `__env__` project (no `project add`, no config.json on disk; token never persisted). Use `--project __env__`. Same env setup also powers `kbagent serve`. kbagent auth login [--stack URL|alias] [--device-code] [--register-projects] +kbagent auth login-password --email EMAIL --password PASSWORD [--totp-secret SECRET] [--stack URL|alias] [--register-projects] kbagent auth status [--stack URL|alias] kbagent auth logout [--stack URL|alias] [--remove-projects] [--yes] kbagent auth register-projects [--stack URL|alias] [--all] [--project-id ID ...] [--alias ID=ALIAS ...] [--yes] +# auth login-password (0.81.0+): the deliberate unattended exception to auth login's "needs a human at +# a browser" rule -- email + password (+ TOTP if the account has MFA) grant, no browser, safe to run +# from a CI secret-backed workflow step. --email/--password/--totp-secret also read from +# KBC_LOGIN_EMAIL/KBC_LOGIN_PASSWORD/KBC_LOGIN_TOTP_SECRET env vars (mirroring KBC_TOKEN's convention), +# so a workflow can set them once in a step's env: block. --totp-secret is the base32 TOTP SEED (not +# a 6-digit code) -- kbagent computes the current code itself (auth/totp.py, stdlib-only RFC 6238), +# so no human ever types a live code. Only the TOTP factor is resolvable this way; a WebAuthn/passkey- +# only account gets AUTH_MFA_INVALID and must use `auth login` (needs a browser) instead. Stores the +# session in auth.json exactly like `auth login` does -- same auth-mode, same "session" column in +# `project list`, same downstream command support. Storing an account's password (and TOTP seed) as +# CI secrets is a bigger blast radius than a single scoped project token: use a dedicated, +# least-privileged service account, never a real human's own credentials. New error code: +# AUTH_MFA_INVALID. # auth (since 0.80.0): browser-based login -- PKCE authorization-code by default (falls back to the # RFC 8628 device flow ONLY on a pre-exchange failure: no loopback browser, callback timeout, or an # SSH/container/WSL heuristic; --device-code forces it). REQUIRES A HUMAN AT A BROWSER -- never attempt -# from an unattended AI agent task; use a static Storage token for CI/headless instead. Issues a +# from an unattended AI agent task; use `auth login-password` or a static Storage token for +# CI/headless instead. Issues a # USER-scoped "programmatic session" (kbc_at_* access token + kbc_rt_* refresh token) stored in # auth.json (0600), a sibling of config.json -- config.json's schema and CURRENT_CONFIG_VERSION are # unchanged. --register-projects writes each accessible project into config.json with the sentinel diff --git a/docs/auth.md b/docs/auth.md index ac4b32bb..6bb0205c 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -7,15 +7,21 @@ token (`kbc_rt_*`) that kbagent renews for you (since v0.80.0). > **Read this first: `auth login` needs a human at a browser.** > -> There is **no headless or unattended path**. `auth login` opens a browser -> window, or prints a code you type into a page on another device. An AI agent -> must never run it on its own initiative — if asked to "set up kbagent auth", -> hand the command back to the person and wait for them to finish. +> There is **no headless or unattended path for `auth login`**. It opens a +> browser window, or prints a code you type into a page on another device. An +> AI agent must never run it on its own initiative — if asked to "set up +> kbagent auth", hand the command back to the person and wait for them to +> finish. > -> For CI, containers, cron, or any other unattended context, use a **static -> Storage token**: `kbagent project add --token ...`, or the token-only -> `KBAGENT_PROJECT_FROM_ENV=1` + `KBC_TOKEN` + `KBC_STORAGE_API_URL` path. -> Neither is affected by anything on this page. +> For CI, containers, cron, or any other unattended context, you have two +> options: a **static Storage token** (`kbagent project add --token ...`, or +> the token-only `KBAGENT_PROJECT_FROM_ENV=1` + `KBC_TOKEN` + +> `KBC_STORAGE_API_URL` path -- unaffected by anything on this page), or +> **`kbagent auth login-password`** (since v0.81.0) if you specifically need a +> full USER-scoped session rather than a single project's token -- see +> [section 2b](#2b-auth-login-password-the-unattended-exception) below. It is +> the one deliberate exception to "no unattended path": it needs an account's +> password (and TOTP seed, if MFA is on) as CI secrets, not a browser. ## TL;DR @@ -107,6 +113,56 @@ kbagent auth login [--stack URL|ALIAS] [--device-code] [--register-projects] below (TTY, non-`--json`), or prints a one-line hint pointing at `auth register-projects`. +### 2b. `auth login-password` -- the unattended exception + +```bash +kbagent auth login-password --email EMAIL --password PASSWORD [--totp-secret SECRET] \ + [--stack URL|alias] [--register-projects] +``` + +The one command in this whole page that IS safe for a CI job or an agent to +run non-interactively -- because it needs credentials handed to it, not a +browser. A password grant, straight to the auth service, no loopback +listener, no user interaction of any kind. + +- **`--email` / `--password` / `--totp-secret`** also read from + `KBC_LOGIN_EMAIL` / `KBC_LOGIN_PASSWORD` / `KBC_LOGIN_TOTP_SECRET` env vars + (the exact convention `KBC_TOKEN` already uses), so a workflow sets them + once in a step's `env:` block instead of passing flags: + ```yaml + - name: Sign in + env: + KBC_LOGIN_EMAIL: ${{ secrets.KBC_LOGIN_EMAIL }} + KBC_LOGIN_PASSWORD: ${{ secrets.KBC_LOGIN_PASSWORD }} + KBC_LOGIN_TOTP_SECRET: ${{ secrets.KBC_LOGIN_TOTP_SECRET }} + run: kbagent auth login-password --register-projects + ``` +- **`--totp-secret` is the account's base32 TOTP *seed*** -- the same string + an authenticator app scans from the enrollment QR code, not a live 6-digit + code. kbagent computes the current code itself (`auth/totp.py`, plain + stdlib RFC 6238 -- no dependency added) at the moment it calls the login + endpoint. Nobody types a live code; the seed is the only secret involved. +- **Only TOTP-based MFA can be resolved this way.** If the account's MFA + factor is WebAuthn/passkey instead, there is no shared secret to compute + a response from -- a WebAuthn ceremony is a live cryptographic exchange + that can only run in a real browser holding the actual passkey/security + key, a hard constraint of the protocol, not a missing feature here -- this + command fails fast with `AUTH_MFA_INVALID` naming `auth login` as the + fallback for that account. +- The resulting session is stored in `auth.json` and behaves **identically** + to a browser-login session from here on: same bearer dispatch, same + refresh rotation, same `--register-projects` contract, same `project list` + `Auth` column (`session`), same [section 4](#4-what-works-on-a-session-project) + restrictions. +- **Security posture matters here more than for a single project's token.** + A password (+ TOTP seed) is the account's full ambient identity, not a + scoped credential -- whoever holds these CI secrets can do anything that + account can do, everywhere it has access, not just one project's Storage + routes. Use a dedicated, least-privileged service account created + specifically for this pipeline; never a real person's own login. Revoking + access means changing that account's password (and re-enrolling MFA), not + a lightweight per-secret revoke. + ### `auth register-projects` ```bash @@ -331,6 +387,7 @@ message text. Full catalogue: | `AUTH_STATE_MISMATCH` | The PKCE callback's `state` did not match the one issued | Re-run `auth login`; if it repeats, something is intercepting the callback | | `SESSION_EXPIRED` | The refresh token expired or was revoked | `kbagent auth login` again — on the host, if this came from `serve` | | `SESSION_NOT_FOUND` | No session is stored for this stack | `kbagent auth login --stack ` | +| `AUTH_MFA_INVALID` | `auth login-password` hit an MFA factor it cannot resolve (e.g. WebAuthn-only) | Use `kbagent auth login` for that account instead | In a multi-project command, a per-project failure appears in the `errors` array of the result envelope with its own `error_code`, so one session project cannot diff --git a/docs/error-codes.md b/docs/error-codes.md index c2483cdc..e6d4fba0 100644 --- a/docs/error-codes.md +++ b/docs/error-codes.md @@ -182,3 +182,4 @@ of `ErrorCode` in `src/keboola_agent_cli/errors.py`. | `AUTH_STATE_MISMATCH` | The PKCE callback's `state` parameter did not match the one generated at login start | | `SESSION_EXPIRED` | The programmatic-auth session's refresh token expired or was revoked; run `kbagent auth login` again | | `SESSION_NOT_FOUND` | No programmatic-auth session is persisted for this stack; run `kbagent auth login` | +| `AUTH_MFA_INVALID` | `auth login-password` hit an MFA factor it cannot resolve without a browser (e.g. WebAuthn-only) -- use `kbagent auth login` for that account instead | diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index bafaef42..6faf77a1 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.83.0", + "version": "0.84.0", "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/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 353039b4..8cb9d904 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -69,6 +69,7 @@ When working inside a git repository or project directory, run `kbagent init` (o | Remove all permission restrictions | `kbagent permissions reset` | | Check if a specific operation is allowed | `kbagent permissions check ` | | Sign in to a Keboola stack via browser login (PKCE) or device code | `kbagent auth login` | +| Sign in via email + password (+ TOTP if the account has MFA) -- no browser | `kbagent auth login-password --email EMAIL --password PASSWORD` | | Show the programmatic-auth session health for a stack | `kbagent auth status` | | Revoke and clear the local programmatic-auth session for a stack | `kbagent auth logout` | | Register accessible projects from the current session as local aliases | `kbagent auth register-projects` | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 2ec571b9..2913bdf5 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -13,14 +13,17 @@ All commands support `--json` for structured output. Multi-project flags (`--pro ## Programmatic Auth (Browser Login) (since v0.80.0) Alternative to a static Storage API token: sign in via a real browser (PKCE -authorization-code) or, when no browser is usable, an RFC 8628 device code. -**Requires a human at a browser/device -- never run `auth login` from an -unattended agent task.** Issues a USER-scoped "programmatic session" +authorization-code) or, when no browser is usable, an RFC 8628 device code -- +or, for CI/automation, a password grant (`auth login-password`, no browser). +**`auth login` requires a human at a browser/device -- never run it from an +unattended agent task; `auth login-password` is the deliberate exception, +safe to run unattended.** Both issue a USER-scoped "programmatic session" (`kbc_at_*` access token + `kbc_rt_*` refresh token) stored in `auth.json` (0600), a sibling of `config.json`; `config.json`'s own schema and `CURRENT_CONFIG_VERSION` are unchanged. - `auth login [--stack URL|alias] [--device-code] [--register-projects]` -- sign in. `--stack` accepts a bare stack URL or an existing project alias (its stack is used); omitted, resolves from the default project's stack. `--device-code` forces the RFC 8628 flow even with a browser available. Without it, PKCE is tried first and falls back to the device flow ONLY on a pre-exchange failure (no loopback browser, callback timeout, or an SSH/container/WSL heuristic) -- once the browser callback succeeds there is no fallback. `--register-projects` writes every project the session can access into `config.json` under the sentinel token `kbc-session://{project_id}` (existing alias for the same project+stack: left alone; pointing elsewhere: skipped with a warning; two accessible projects sharing the same name now get distinct suggested aliases via a project-id suffix instead of the second one being silently skipped). Without `--register-projects`, on a TTY with human output, `login` offers the same picker interactively right after reporting success -- see `auth register-projects` below. +- `auth login-password --email EMAIL --password PASSWORD [--totp-secret SECRET] [--stack URL|alias] [--register-projects]` -- sign in via a password grant, no browser. `--email`/`--password`/`--totp-secret` also read from `KBC_LOGIN_EMAIL`/`KBC_LOGIN_PASSWORD`/`KBC_LOGIN_TOTP_SECRET` env vars (same convention as `KBC_TOKEN`), so a CI workflow sets them once in a step's `env:` block. `--totp-secret` is the account's base32 TOTP seed (from its authenticator enrollment), NOT a 6-digit code -- kbagent computes the current code itself (`auth/totp.py`, stdlib RFC 6238), so nothing here needs a human typing a live code. Only resolves TOTP-based MFA; a WebAuthn/passkey-only account gets `AUTH_MFA_INVALID` and must use `auth login` instead (that ceremony needs a real browser). The resulting session is stored and used identically to a browser-login session -- same `auth.json`, same `project list` "session" auth-mode, same `--register-projects` contract. Storing an account's password (and TOTP seed) as CI secrets is a bigger blast radius than one scoped project token; use a dedicated, least-privileged service account. - `auth status [--stack URL|alias]` -- show session state (`live`/`refreshed`/`degraded`/`expired`/`missing`), signed-in user, accessible projects, and token expiry. Proactively refreshes the access token if stale before reporting (a healthy session routinely shows an expired 1h access token next to a valid 30-day refresh token) -- `refreshed` means a rotation just happened, `live` means the cached token was still fresh. - `auth logout [--stack URL|alias] [--remove-projects] [--yes]` -- revoke the refresh token server-side and delete the local session from `auth.json`. `--remove-projects` also removes `config.json` aliases pointing at this session (sentinel-token projects only; a static-token project on the same stack is never touched). - `auth register-projects [--stack URL|alias] [--all] [--project-id ID ...] [--alias ID=ALIAS ...] [--yes]` -- register an EXISTING session's accessible projects as `config.json` aliases, without re-running `login`. Fixes two usability gaps in plain `login`: nothing was registered unless `--register-projects` was passed, and the suggested alias was always slugified from the project NAME, so a project id like `9840` from the login table never resolved as `--project 9840`. `--all` selects every accessible project; `--project-id ID` (repeatable) selects specific ones (an inaccessible id raises a `ConfigError`); passing neither starts an interactive arrow-key + spacebar checkbox picker -- every not-yet-registered project preselected, up/down or `j`/`k` move, `space` toggles, `a` selects/deselects all, `enter` accepts, `q`/`esc`/`ctrl-c` cancels -- followed by a single `Edit aliases?` confirm (default no) that opens the old per-project alias prompt only if you opt in (each row already shows its suggested alias), then a final `typer.confirm`. On a piped stdin or a terminal without real interactive capabilities, the picker falls back to the original typed prompt (numbers / ranges `1-3` / `all` / `none`). In a non-TTY or `--json` context with neither `--all` nor `--project-id`, the command fails fast telling the caller to pass `--all` or `--project-id` instead of hanging on a prompt. `--alias ID=ALIAS` (repeatable) overrides the suggested alias for a given project id in every mode, including as the picker's prefilled default. `--yes` skips only the picker's final confirmation. Two collision rules, in both modes: a project already registered under an alias for this project+stack reports `status: "exists"` (no-op -- rename via `project edit --new-alias` instead of re-registering); an alias already claimed by a different project (or a static-token project) reports `status: "skipped"` with a rename-hint note -- an existing `config.json` entry is never overwritten. `auth login` (without `--register-projects`) now also offers this same picker interactively right after a successful login, when stdout is a TTY and `--json` was not used; otherwise it just prints the hint to run this command later, and a failure in that optional follow-up never changes `login`'s own (already-successful) exit code. diff --git a/pyproject.toml b/pyproject.toml index 063cfc9e..67b034b4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-cli" -version = "0.83.0" +version = "0.84.0" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/auth/auth_client.py b/src/keboola_agent_cli/auth/auth_client.py index e3afacba..0dd87a61 100644 --- a/src/keboola_agent_cli/auth/auth_client.py +++ b/src/keboola_agent_cli/auth/auth_client.py @@ -33,6 +33,8 @@ AUTH_CLIENT_ID, AUTH_DEVICE_PATH, AUTH_DEVICE_TOKEN_PATH, + AUTH_LOGIN_PATH, + AUTH_MFA_PATH, AUTH_PKCE_AUTHORIZE_PATH, AUTH_PKCE_TOKEN_PATH, AUTH_REFRESH_CONTENTION_DEFAULT_DELAY, @@ -54,6 +56,7 @@ DevicePollResult, DevicePollStatus, IntrospectResponse, + MfaChallengeResult, RevokeResult, ) @@ -258,6 +261,33 @@ def __exit__(self, *args: Any) -> None: # PKCE # ------------------------------------------------------------------ + def login_password(self, email: str, password: str) -> CliTokenResponse | MfaChallengeResult: + """Password-grant login (``POST /v1/auth/login``). + + The unattended-capable login path -- unlike PKCE/device, this never + opens a browser. Returns a token pair directly, or an + `MfaChallengeResult` if the account has MFA configured (resolve it + with `verify_mfa_totp`). + """ + response = self._do_request( + "POST", + AUTH_LOGIN_PATH, + json={"grantType": "password", "email": email, "password": password}, + ) + data = response.json() + if data.get("mfaRequired"): + return MfaChallengeResult.model_validate(data) + return CliTokenResponse.model_validate(data) + + def verify_mfa_totp(self, mfa_token: str, code: str) -> CliTokenResponse: + """Resolve a password-login MFA challenge via TOTP (``POST /v1/auth/mfa``).""" + response = self._do_request( + "POST", + AUTH_MFA_PATH, + json={"mfaToken": mfa_token, "type": "totp", "code": code}, + ) + return CliTokenResponse.model_validate(response.json()) + def authorize_url(self, *, redirect_uri: str, code_challenge: str, state: str) -> str: """Build the browser-facing PKCE authorize URL. diff --git a/src/keboola_agent_cli/auth/models.py b/src/keboola_agent_cli/auth/models.py index 935bdaf9..6448cb10 100644 --- a/src/keboola_agent_cli/auth/models.py +++ b/src/keboola_agent_cli/auth/models.py @@ -123,6 +123,24 @@ class DeviceAuthorization(BaseModel): model_config = _WIRE_MODEL_CONFIG +class MfaChallengeResult(BaseModel): + """Response to POST /v1/auth/login when the account has MFA configured. + + Returned instead of a token pair -- resolve it via POST /v1/auth/mfa + (`AuthClient.verify_mfa_totp` for the TOTP factor; WebAuthn/recovery-code + are not wired here, since `login_password` exists specifically for the + no-browser CI path and WebAuthn needs one). + """ + + mfa_required: bool = Field(default=True, alias="mfaRequired") + mfa_type: str = Field(default="", alias="mfaType") + mfa_token: str = Field(default="", alias="mfaToken") + expires_in: int = Field(default=0, alias="expiresIn") + allowed_methods: list[str] = Field(default_factory=list, alias="allowedMethods") + + model_config = _WIRE_MODEL_CONFIG + + class AuthProject(BaseModel): """One project accessible to the signed-in session, from introspect.""" diff --git a/src/keboola_agent_cli/auth/totp.py b/src/keboola_agent_cli/auth/totp.py new file mode 100644 index 00000000..fd353cd2 --- /dev/null +++ b/src/keboola_agent_cli/auth/totp.py @@ -0,0 +1,26 @@ +"""RFC 6238 TOTP code computation, stdlib only. + +Used by `login_password` to resolve a TOTP MFA challenge non-interactively +(the whole point of the password-grant login path -- see +`services/auth_service.py`'s `login_password`): given the account's base32 +TOTP seed (stored as a CI secret alongside the password), compute the +current 6-digit code instead of prompting a human for one. +""" + +from __future__ import annotations + +import base64 +import hashlib +import hmac +import struct +import time + + +def compute_totp_code(secret_b32: str, *, digits: int = 6, period: int = 30) -> str: + """Compute the current TOTP code for a base32-encoded secret (RFC 6238).""" + key = base64.b32decode(secret_b32.strip().upper().replace(" ", ""), casefold=True) + counter = int(time.time() // period) + digest = hmac.new(key, struct.pack(">Q", counter), hashlib.sha1).digest() + offset = digest[-1] & 0x0F + code = (struct.unpack(">I", digest[offset : offset + 4])[0] & 0x7FFFFFFF) % (10**digits) + return str(code).zfill(digits) diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index a233146d..7c0da112 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -24,6 +24,23 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.84.0": [ + "New: `kbagent auth login-password --email EMAIL (--password PASSWORD | " + "--password-stdin) [--totp-secret SECRET]` -- the deliberate unattended exception " + 'to `auth login`\'s "needs a human at a browser" rule. A password grant (no ' + "browser), safe to run from a CI secret-backed workflow step. `--password-stdin` " + "(hidden prompt on a TTY, reads to EOF on a pipe) is the recommended way to supply " + "the password -- `--password` and `--password-stdin` are mutually exclusive. " + "`--email`/`--password`/`--totp-secret` also read from " + "`KBC_LOGIN_EMAIL`/`KBC_LOGIN_PASSWORD`/`KBC_LOGIN_TOTP_SECRET` env vars (same " + "convention as `KBC_TOKEN`). `--totp-secret` is the account's base32 TOTP seed, " + "not a 6-digit code -- kbagent computes the current code itself (stdlib RFC 6238), " + "so no human ever types a live code. Only resolves TOTP-based MFA; a " + "WebAuthn/passkey-only account gets the new `AUTH_MFA_INVALID` error and must use " + "`auth login` instead. Stores the resulting session in `auth.json` exactly like " + "`auth login` does -- same downstream command support, same " + "`--register-projects` contract.", + ], "0.83.0": [ "Note (#390): there will be NO `kbagent agent migrate-mcp-tasks` command. Migrating " "a scheduled `--type mcp_tool` task to `--type cli_command` before v0.85.0 is a " diff --git a/src/keboola_agent_cli/commands/auth.py b/src/keboola_agent_cli/commands/auth.py index 8b493cb0..de38a3b3 100644 --- a/src/keboola_agent_cli/commands/auth.py +++ b/src/keboola_agent_cli/commands/auth.py @@ -1,4 +1,4 @@ -"""Programmatic browser login -- `kbagent auth login|status|logout|register-projects`. +"""Programmatic browser login -- `kbagent auth login|login-password|status|logout|register-projects`. Thin CLI layer for the `kbagent auth` command group: parses arguments, calls :class:`AuthService`, formats output. No business logic belongs here -- alias @@ -6,13 +6,14 @@ live in `AuthService` / `config_store.py`. The interactive picker itself lives in `_auth_picker.py` (terminal I/O only, same reasoning). -Signs in a user-scoped Keboola session (PKCE authorization-code flow, or a -device-code flow for headless/remote machines) and stores it in `auth.json`. -Requires a human at a browser (or able to visit a URL and type a code) -- -this is not something an AI agent can complete unattended. The resulting -session tokens are never printed or retrievable via the CLI; every result -below is built from a dataclass with no token field, so `--json` output is -safe by construction. +Two ways to sign in, stored the same way in `auth.json`: `login` (PKCE +authorization-code, or a device-code flow for headless/remote machines) +requires a human at a browser -- an AI agent must not attempt it on its own +initiative. `login-password` (email + password + TOTP) is the deliberate +unattended exception, built for CI: it never opens a browser and is safe to +run from a secret-backed workflow step. The resulting session tokens are +never printed or retrievable via the CLI; every result below is built from +a dataclass with no token field, so `--json` output is safe by construction. """ from __future__ import annotations @@ -28,6 +29,8 @@ from rich.table import Table from ..auth.models import DeviceAuthorization +from ..auth.totp import compute_totp_code +from ..constants import ENV_KBC_LOGIN_EMAIL, ENV_KBC_LOGIN_PASSWORD, ENV_KBC_LOGIN_TOTP_SECRET from ..errors import ConfigError, ErrorCode, KeboolaApiError from ..output import OutputFormatter from ..services.auth_service import ( @@ -382,6 +385,76 @@ def _run_post_login_hook( ) +@auth_app.command("login-password") +def auth_login_password( + ctx: typer.Context, + email: str = typer.Option( + ..., + "--email", + envvar=ENV_KBC_LOGIN_EMAIL, + help="Account email. Also settable via KBC_LOGIN_EMAIL.", + ), + password: str = typer.Option( + ..., + "--password", + envvar=ENV_KBC_LOGIN_PASSWORD, + help="Account password. Prefer KBC_LOGIN_PASSWORD (a CI secret in the step's " + "env: block) over typing this flag directly -- it avoids the value landing in " + "shell history or a process listing.", + ), + totp_secret: str | None = typer.Option( + None, + "--totp-secret", + envvar=ENV_KBC_LOGIN_TOTP_SECRET, + help="Base32 TOTP seed (from the account's authenticator enrollment, NOT a " + "6-digit code) -- required if the account has TOTP-based MFA configured. " + "kbagent computes the current code from this itself, so no human ever " + "types a live code. Also settable via KBC_LOGIN_TOTP_SECRET.", + ), + stack: str | None = typer.Option( + None, "--stack", help="Stack URL or a registered project alias to log into" + ), + register_projects: bool = typer.Option( + False, + "--register-projects", + help="Register every project this session can access under a local alias", + ), +) -> None: + """Sign in via email + password (+ TOTP if the account has MFA) -- no browser. + + This is `auth login`'s unattended counterpart, built for CI: it never + opens a browser and completes entirely over HTTP, so it is safe to run + from a secret-backed workflow step. It does NOT relax `auth login`'s own + "requires a human at a browser" contract for the PKCE/device flows -- + this is a separate, explicitly opt-in command using a different grant + entirely. + + Only TOTP-based MFA can be resolved here (kbagent computes the current + code itself from --totp-secret). An account with WebAuthn/passkey-only + MFA cannot use this command -- that ceremony needs a real browser; use + `kbagent auth login` for such an account instead. + + Storing an account's password (and TOTP seed) as CI secrets is a bigger + blast radius than a single scoped credential: whoever holds them can do + anything that account can do, not just what one project's token allows. + Use a dedicated, least-privileged service account for this, never a + real human's own credentials. + """ + formatter = get_formatter(ctx) + service: AuthService = get_service(ctx, "auth_service") + try: + result = service.login_password( + stack=stack, + email=email, + password=password, + totp_code=compute_totp_code(totp_secret) if totp_secret else None, + register_projects=register_projects, + ) + except (ConfigError, KeboolaApiError) as exc: + _handle_errors(formatter, exc) + formatter.output(result, _format_login_result) + + @auth_app.command("status") def auth_status( ctx: typer.Context, diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index f77f1c49..5deb9f11 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -60,11 +60,14 @@ authorization on SSH/containers/WSL, or with --device-code) as an alternative to a long-lived static Storage API token. - IMPORTANT FOR AI AGENTS: `auth login` REQUIRES A HUMAN AT A BROWSER. - There is no unattended/headless path, and session tokens are deliberately - not readable through the CLI -- do NOT attempt this command from an - unattended agent task. For headless / CI / automation use, keep using a - static Storage token (`project add --token` or `KBAGENT_PROJECT_FROM_ENV`). + IMPORTANT FOR AI AGENTS: `auth login` (PKCE / device flow) REQUIRES A + HUMAN AT A BROWSER. There is no unattended path for THAT command, and + session tokens are deliberately not readable through the CLI -- do NOT + attempt `auth login` from an unattended agent task. `auth login-password` + (below) is the one deliberate, explicit exception: it is safe to run + unattended given the credentials it needs, and is what CI/automation + should use to get a full session (as opposed to a single-project static + Storage token via `project add --token` / `KBAGENT_PROJECT_FROM_ENV`). kbagent auth login [--stack URL|alias] [--device-code] [--register-projects] Opens the Keboola login page in a browser (or falls back to the RFC 8628 @@ -94,6 +97,27 @@ A 404 from any auth endpoint means browser login is not enabled on that stack yet (per-stack feature flag); use a static token instead. + kbagent auth login-password --email EMAIL --password PASSWORD [--totp-secret SECRET] [--stack URL|alias] [--register-projects] + Sign in via a password grant -- no browser, safe to run unattended from + a CI secret-backed workflow. --email/--password/--totp-secret also read + from KBC_LOGIN_EMAIL/KBC_LOGIN_PASSWORD/KBC_LOGIN_TOTP_SECRET env vars + (same convention as KBC_TOKEN), so a workflow sets them once in a + step's env: block instead of passing flags. --totp-secret is the + account's base32 TOTP SEED (from its authenticator enrollment), NOT a + 6-digit code -- kbagent computes the current code itself + (auth/totp.py, stdlib RFC 6238) so no human ever types a live code. + Only resolves TOTP-based MFA this way; a WebAuthn/passkey-only account + gets AUTH_MFA_INVALID and must use `auth login` instead (that ceremony + needs a real browser). Stores the resulting session in auth.json + exactly like `auth login` does -- same downstream command support, + same `project list` "session" auth-mode column, same + --register-projects contract. AN AI AGENT MAY run this command when + given real credentials for this purpose (unlike `auth login`), but + must never invent, guess, or reuse credentials from another context. + Storing an account's password (and TOTP seed) as CI secrets is a + bigger blast radius than one scoped project token -- use a dedicated, + least-privileged service account, never a real human's own login. + kbagent auth status [--stack URL|alias] Show the current session's state (live/refreshed/degraded/expired/missing), signed-in user, accessible projects, and token expiry. Proactively diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 7c80c609..a1d08041 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -224,6 +224,12 @@ def _resolve_app_name() -> str: ENV_KBC_STORAGE_API_URL: str = "KBC_STORAGE_API_URL" ENV_KBC_MANAGE_API_TOKEN: str = "KBC_MANAGE_API_TOKEN" ENV_KBC_MASTER_TOKEN: str = "KBC_MASTER_TOKEN" +# `auth login-password` (since 0.81.0) -- the unattended, CI-safe login path. +# Mirrors KBC_TOKEN's env-injection convention so a workflow can set these +# once in a step's `env:` block instead of piping secrets through stdin/files. +ENV_KBC_LOGIN_EMAIL: str = "KBC_LOGIN_EMAIL" +ENV_KBC_LOGIN_PASSWORD: str = "KBC_LOGIN_PASSWORD" +ENV_KBC_LOGIN_TOTP_SECRET: str = "KBC_LOGIN_TOTP_SECRET" ENV_MCP_TOOL_TIMEOUT: str = "KBAGENT_MCP_TOOL_TIMEOUT" ENV_MCP_INIT_TIMEOUT: str = "KBAGENT_MCP_INIT_TIMEOUT" ENV_MCP_MAX_SESSIONS: str = "KBAGENT_MCP_MAX_SESSIONS" @@ -645,6 +651,10 @@ def _resolve_app_name() -> str: SESSION_TOKEN_PREFIX: str = "kbc-session://" # Server endpoints, relative to the stack base URL. +# Password-grant login (since 0.81.0) -- the CI-safe, unattended alternative +# to the browser-only PKCE/device flows. See AuthService.login_password. +AUTH_LOGIN_PATH: str = "/v1/auth/login" +AUTH_MFA_PATH: str = "/v1/auth/mfa" AUTH_PKCE_AUTHORIZE_PATH: str = "/admin/auth/pkce/authorize" AUTH_PKCE_TOKEN_PATH: str = "/v1/auth/pkce/token" AUTH_DEVICE_PATH: str = "/v1/auth/device" diff --git a/src/keboola_agent_cli/errors.py b/src/keboola_agent_cli/errors.py index bfe912cb..2b6622a1 100644 --- a/src/keboola_agent_cli/errors.py +++ b/src/keboola_agent_cli/errors.py @@ -137,6 +137,9 @@ class ErrorCode(StrEnum): SESSION_EXPIRED = "SESSION_EXPIRED" SESSION_NOT_FOUND = "SESSION_NOT_FOUND" + # Password-grant login (since 0.81.0) + AUTH_MFA_INVALID = "AUTH_MFA_INVALID" + def mask_token(token: str) -> str: """Mask a Keboola Storage API token for safe display. @@ -317,6 +320,7 @@ def __init__(self, feature: str, *, remedy: str = "") -> None: ErrorCode.AUTH_STATE_MISMATCH: "authentication", ErrorCode.SESSION_EXPIRED: "authentication", ErrorCode.SESSION_NOT_FOUND: "authentication", + ErrorCode.AUTH_MFA_INVALID: "authentication", } diff --git a/src/keboola_agent_cli/permissions.py b/src/keboola_agent_cli/permissions.py index a3d54c67..3f376f5b 100644 --- a/src/keboola_agent_cli/permissions.py +++ b/src/keboola_agent_cli/permissions.py @@ -19,6 +19,7 @@ # read-only local + introspect check. `auth logout --remove-projects` does # touch the registry and is escalated via FLAG_ESCALATIONS below. "auth.login": "write", + "auth.login-password": "write", "auth.logout": "write", "auth.status": "read", # register-projects only ever writes config.json (session-sentinel diff --git a/src/keboola_agent_cli/services/auth_service.py b/src/keboola_agent_cli/services/auth_service.py index 817d319d..0841bcbe 100644 --- a/src/keboola_agent_cli/services/auth_service.py +++ b/src/keboola_agent_cli/services/auth_service.py @@ -21,7 +21,7 @@ from ..auth.auth_client import AuthClient from ..auth.device import run_device_flow from ..auth.environment import BrowserEnvironment, detect_browser_environment, open_browser -from ..auth.models import CliTokenResponse, DeviceAuthorization, StackSession +from ..auth.models import CliTokenResponse, DeviceAuthorization, MfaChallengeResult, StackSession from ..auth.pkce import ( PkceAuthorizationError, PkceCallbackServer, @@ -270,92 +270,165 @@ def login( "Login flow completed without producing a token pair.", error_code=ErrorCode.API_ERROR, ) - now = datetime.now(UTC) - previous = self._state_store.get_session(stack_url) - new_session = StackSession( - stack_url=stack_url, - session_id=tokens.session_id, - user_email=tokens.user.email if tokens.user else "", - user_name=tokens.user.name if tokens.user else "", - access_token=tokens.access_token, - refresh_token=tokens.refresh_token, - access_expires_at=now + timedelta(seconds=tokens.expires_in), - # Read through the shared model helper, not hardcoded None: no - # backend sends a refresh expiry today, but if one starts, - # honouring it here (rather than only on the first refresh) - # keeps login and rotation consistent. See - # `CliTokenResponse.refresh_expiry` for why nothing is guessed - # when the field is absent. - refresh_expires_at=tokens.refresh_expiry(now=now), - created_at=now, - # An orphan is a session this CLI failed to revoke server-side, - # and the only record that it exists. `put_session` replaces the - # whole per-stack row, so a login that did not carry the list - # forward would drop every orphan older than the session it is - # replacing -- leaving a live session no `auth logout` can ever - # reach, while telling the user logout would retry it. - orphaned_session_ids=list(previous.orphaned_session_ids) if previous else [], + return self._finalize_login( + client, + stack_url, + tokens, + method=method, + fallback_reason=fallback_reason, + register_projects=register_projects, + warnings=warnings, ) - # Durable first, always -- never delete the old credentials before - # the new ones are safely on disk (review B-1). - self._state_store.put_session(new_session) - - replaced_session_id = "" - orphaned_session_id = "" - if previous is not None and previous.session_id != new_session.session_id: - replaced_session_id = previous.session_id - revoke_result = client.revoke( - previous.refresh_token, token_type_hint="refreshToken" - ) - if not revoke_result.confirmed: - orphaned_session_id = previous.session_id - self._state_store.record_orphan(stack_url, previous.session_id) - warnings.append( - f"Could not confirm revocation of the previous session " - f"({previous.session_id}); it may still be active on the " - "server. `kbagent auth logout` will retry it." + def login_password( + self, + *, + stack: str | None = None, + email: str, + password: str, + totp_code: str | None = None, + register_projects: bool = False, + ) -> LoginResult: + """Password-grant login -- the unattended, CI-safe alternative to `login()`. + + Never opens a browser and completes entirely over HTTP, so it is + safe to run from a secret-backed CI workflow (unlike `login()`, + which requires a human at a browser or device). `totp_code` resolves + an MFA challenge for an account with TOTP-based MFA configured; + WebAuthn-only accounts cannot use this method -- that ceremony + needs a browser, which is exactly what this path exists to avoid. + + The rest of the algorithm (session persistence, best-effort revoke + of the session it replaces, introspection, optional project + registration) is identical to `login()` -- see `_finalize_login`. + """ + stack_url = self._resolve_stack_url(stack) + warnings: list[str] = [] + with self._auth_client_factory(stack_url) as client: + result = client.login_password(email, password) + if isinstance(result, MfaChallengeResult): + if result.mfa_type != "totp": + raise KeboolaApiError( + f"This account requires MFA type {result.mfa_type!r}, which " + "password-grant login cannot resolve without a browser -- use " + "`kbagent auth login` instead.", + error_code=ErrorCode.AUTH_MFA_INVALID, + retryable=False, + ) + if not totp_code: + raise ConfigError( + "This account requires a TOTP code to sign in -- pass totp_code." ) + tokens = client.verify_mfa_totp(result.mfa_token, totp_code) + else: + tokens = result + return self._finalize_login( + client, + stack_url, + tokens, + method="password", + fallback_reason="", + register_projects=register_projects, + warnings=warnings, + ) - introspection = client.introspect(new_session.access_token) - accessible_projects = [ - {"id": project.id, "name": project.name, "role": project.role} - for project in introspection.projects - ] - - registered_projects: list[RegisteredProject] = [] - if register_projects: - # Build candidates from the introspection this call already - # holds -- introspecting a second time (e.g. via - # `register_projects`) would be a redundant network round - # trip against a session that was only just minted. - candidates = self.candidates_from_projects(stack_url, accessible_projects) - selections = [ProjectSelection(project_id=c.project_id) for c in candidates] - registered_projects = apply_selections( - self._config_store, - stack_url, - {c.project_id: c for c in candidates}, - selections, - warnings, + def _finalize_login( + self, + client: AuthClient, + stack_url: str, + tokens: CliTokenResponse, + *, + method: str, + fallback_reason: str, + register_projects: bool, + warnings: list[str], + ) -> LoginResult: + """Shared tail of every login method: persist, best-effort revoke the + session this replaces, introspect, optionally register projects.""" + now = datetime.now(UTC) + previous = self._state_store.get_session(stack_url) + new_session = StackSession( + stack_url=stack_url, + session_id=tokens.session_id, + user_email=tokens.user.email if tokens.user else "", + user_name=tokens.user.name if tokens.user else "", + access_token=tokens.access_token, + refresh_token=tokens.refresh_token, + access_expires_at=now + timedelta(seconds=tokens.expires_in), + # Read through the shared model helper, not hardcoded None: no + # backend sends a refresh expiry today, but if one starts, + # honouring it here (rather than only on the first refresh) + # keeps login and rotation consistent. See + # `CliTokenResponse.refresh_expiry` for why nothing is guessed + # when the field is absent. + refresh_expires_at=tokens.refresh_expiry(now=now), + created_at=now, + # An orphan is a session this CLI failed to revoke server-side, + # and the only record that it exists. `put_session` replaces the + # whole per-stack row, so a login that did not carry the list + # forward would drop every orphan older than the session it is + # replacing -- leaving a live session no `auth logout` can ever + # reach, while telling the user logout would retry it. + orphaned_session_ids=list(previous.orphaned_session_ids) if previous else [], + ) + + # Durable first, always -- never delete the old credentials before + # the new ones are safely on disk (review B-1). + self._state_store.put_session(new_session) + + replaced_session_id = "" + orphaned_session_id = "" + if previous is not None and previous.session_id != new_session.session_id: + replaced_session_id = previous.session_id + revoke_result = client.revoke(previous.refresh_token, token_type_hint="refreshToken") + if not revoke_result.confirmed: + orphaned_session_id = previous.session_id + self._state_store.record_orphan(stack_url, previous.session_id) + warnings.append( + f"Could not confirm revocation of the previous session " + f"({previous.session_id}); it may still be active on the " + "server. `kbagent auth logout` will retry it." ) - return LoginResult( - status="ok", - method=method, - stack_url=stack_url, - session_id=new_session.session_id, - user_email=new_session.user_email, - user_name=new_session.user_name, - access_expires_at=_iso(new_session.access_expires_at), - refresh_expires_at=_iso(new_session.refresh_expires_at), - fallback_reason=fallback_reason, - replaced_session_id=replaced_session_id, - orphaned_session_id=orphaned_session_id, - accessible_projects=accessible_projects, - registered_projects=registered_projects, - warnings=warnings, + introspection = client.introspect(new_session.access_token) + accessible_projects = [ + {"id": project.id, "name": project.name, "role": project.role} + for project in introspection.projects + ] + + registered_projects: list[RegisteredProject] = [] + if register_projects: + # Build candidates from the introspection this call already + # holds -- introspecting a second time (e.g. via + # `register_projects`) would be a redundant network round + # trip against a session that was only just minted. + candidates = self.candidates_from_projects(stack_url, accessible_projects) + selections = [ProjectSelection(project_id=c.project_id) for c in candidates] + registered_projects = apply_selections( + self._config_store, + stack_url, + {c.project_id: c for c in candidates}, + selections, + warnings, ) + return LoginResult( + status="ok", + method=method, + stack_url=stack_url, + session_id=new_session.session_id, + user_email=new_session.user_email, + user_name=new_session.user_name, + access_expires_at=_iso(new_session.access_expires_at), + refresh_expires_at=_iso(new_session.refresh_expires_at), + fallback_reason=fallback_reason, + replaced_session_id=replaced_session_id, + orphaned_session_id=orphaned_session_id, + accessible_projects=accessible_projects, + registered_projects=registered_projects, + warnings=warnings, + ) + def _wrap_device_prompt( self, prompt: Callable[[DeviceAuthorization], None], diff --git a/tests/test_auth_client.py b/tests/test_auth_client.py index 9a7a4d31..ddbbe3a2 100644 --- a/tests/test_auth_client.py +++ b/tests/test_auth_client.py @@ -28,6 +28,7 @@ DeviceAuthorization, DevicePollStatus, IntrospectResponse, + MfaChallengeResult, RevokeResult, ) from keboola_agent_cli.commands._helpers import map_error_to_exit_code @@ -135,6 +136,109 @@ def test_sends_exact_body_and_parses_response(self, httpx_mock) -> None: } +# ---------------------------------------------------------------------------- +# Password-grant login +# ---------------------------------------------------------------------------- + + +class TestLoginPassword: + def test_no_mfa_returns_token_pair(self, httpx_mock) -> None: + httpx_mock.add_response( + url=f"{STACK_URL}/v1/auth/login", + method="POST", + status_code=200, + json={ + "accessToken": "kbc_at_abc", + "refreshToken": "kbc_rt_def", + "tokenType": "Bearer", + "expiresIn": 3600, + "sessionId": "sess-1", + "user": {"id": 42, "email": "svc@example.com", "name": "Service"}, + }, + ) + client = _make_client() + try: + result = client.login_password("svc@example.com", "s3cr3t") + finally: + client.close() + + assert isinstance(result, CliTokenResponse) + assert result.access_token == "kbc_at_abc" + + request = httpx_mock.get_requests()[0] + assert json.loads(request.read().decode()) == { + "grantType": "password", + "email": "svc@example.com", + "password": "s3cr3t", + } + assert "Authorization" not in request.headers + + def test_mfa_required_returns_challenge(self, httpx_mock) -> None: + httpx_mock.add_response( + url=f"{STACK_URL}/v1/auth/login", + method="POST", + status_code=200, + json={ + "mfaRequired": True, + "mfaType": "totp", + "mfaToken": "kbc_mfa_xyz", + "expiresIn": 300, + "allowedMethods": ["totp", "recovery_code"], + }, + ) + client = _make_client() + try: + result = client.login_password("svc@example.com", "s3cr3t") + finally: + client.close() + + assert isinstance(result, MfaChallengeResult) + assert result.mfa_type == "totp" + assert result.mfa_token == "kbc_mfa_xyz" + + def test_404_maps_to_auth_not_supported(self, httpx_mock) -> None: + httpx_mock.add_response( + url=f"{STACK_URL}/v1/auth/login", method="POST", status_code=404, json={} + ) + client = _make_client() + try: + with pytest.raises(KeboolaApiError) as excinfo: + client.login_password("svc@example.com", "s3cr3t") + finally: + client.close() + assert excinfo.value.error_code == ErrorCode.AUTH_NOT_SUPPORTED_ON_STACK + + +class TestVerifyMfaTotp: + def test_sends_type_and_code_returns_tokens(self, httpx_mock) -> None: + httpx_mock.add_response( + url=f"{STACK_URL}/v1/auth/mfa", + method="POST", + status_code=200, + json={ + "accessToken": "kbc_at_abc", + "refreshToken": "kbc_rt_def", + "expiresIn": 3600, + "sessionId": "sess-1", + }, + ) + client = _make_client() + try: + result = client.verify_mfa_totp("kbc_mfa_xyz", "123456") + finally: + client.close() + + assert isinstance(result, CliTokenResponse) + assert result.access_token == "kbc_at_abc" + + request = httpx_mock.get_requests()[0] + assert json.loads(request.read().decode()) == { + "mfaToken": "kbc_mfa_xyz", + "type": "totp", + "code": "123456", + } + + # ---------------------------------------------------------------------------- # Device authorization start # ---------------------------------------------------------------------------- diff --git a/tests/test_auth_service.py b/tests/test_auth_service.py index 8b8a02d1..eca0a3f9 100644 --- a/tests/test_auth_service.py +++ b/tests/test_auth_service.py @@ -22,6 +22,7 @@ CliTokenResponse, DeviceAuthorization, IntrospectResponse, + MfaChallengeResult, RevokeResult, ) from keboola_agent_cli.auth.pkce import ( @@ -67,6 +68,10 @@ def __init__(self) -> None: self.refresh_side_effect: Exception | None = None self.revoke_result = RevokeResult(confirmed=True) self.delete_session_result = RevokeResult(confirmed=True) + self.login_password_response: CliTokenResponse | MfaChallengeResult | None = None + self.login_password_side_effect: Exception | None = None + self.verify_mfa_response: CliTokenResponse | None = None + self.verify_mfa_side_effect: Exception | None = None def __enter__(self) -> _FakeAuthClient: return self @@ -102,6 +107,20 @@ def refresh(self, refresh_token: str) -> CliTokenResponse: assert self.refresh_response is not None return self.refresh_response + def login_password(self, email: str, password: str): + self.calls.append(("login_password", (email, password))) + if self.login_password_side_effect is not None: + raise self.login_password_side_effect + assert self.login_password_response is not None + return self.login_password_response + + def verify_mfa_totp(self, mfa_token: str, code: str) -> CliTokenResponse: + self.calls.append(("verify_mfa_totp", (mfa_token, code))) + if self.verify_mfa_side_effect is not None: + raise self.verify_mfa_side_effect + assert self.verify_mfa_response is not None + return self.verify_mfa_response + def revoke(self, token: str, *, token_type_hint: str = "refreshToken") -> RevokeResult: self.calls.append(("revoke", (token, token_type_hint))) return self.revoke_result @@ -1243,3 +1262,61 @@ def delete_session(self, session_id: str, access_token: str) -> RevokeResult: assert outcome.revoked == ["gone"] assert outcome.remaining == ["stuck"] + + +class TestLoginPassword: + def test_no_mfa_finalizes_like_any_other_login(self, store, state_store) -> None: + client = _FakeAuthClient() + client.login_password_response = _tokens(session_id="sess-pw") + client.introspect_response = _introspect() + service = _make_service(store, state_store, client) + + result = service.login_password(stack=STACK_URL, email="svc@example.com", password="s3cr3t") + + assert result.method == "password" + assert result.session_id == "sess-pw" + assert client.calls[0] == ("login_password", ("svc@example.com", "s3cr3t")) + assert not any(c[0] == "verify_mfa_totp" for c in client.calls) + assert state_store.get_session(STACK_URL) is not None + + def test_totp_challenge_resolved_before_finalizing(self, store, state_store) -> None: + client = _FakeAuthClient() + client.login_password_response = MfaChallengeResult( + mfaRequired=True, mfaType="totp", mfaToken="kbc_mfa_xyz", allowedMethods=["totp"] + ) + client.verify_mfa_response = _tokens(session_id="sess-pw") + client.introspect_response = _introspect() + service = _make_service(store, state_store, client) + + result = service.login_password( + stack=STACK_URL, email="svc@example.com", password="s3cr3t", totp_code="123456" + ) + + assert result.method == "password" + assert client.calls[0] == ("login_password", ("svc@example.com", "s3cr3t")) + assert client.calls[1] == ("verify_mfa_totp", ("kbc_mfa_xyz", "123456")) + + def test_totp_challenge_without_code_raises_config_error(self, store, state_store) -> None: + client = _FakeAuthClient() + client.login_password_response = MfaChallengeResult( + mfaRequired=True, mfaType="totp", mfaToken="kbc_mfa_xyz", allowedMethods=["totp"] + ) + service = _make_service(store, state_store, client) + + with pytest.raises(ConfigError): + service.login_password(stack=STACK_URL, email="svc@example.com", password="s3cr3t") + assert not any(c[0] == "verify_mfa_totp" for c in client.calls) + + def test_webauthn_challenge_raises_mfa_invalid(self, store, state_store) -> None: + client = _FakeAuthClient() + client.login_password_response = MfaChallengeResult( + mfaRequired=True, + mfaType="webauthn", + mfaToken="kbc_mfa_xyz", + allowedMethods=["webauthn"], + ) + service = _make_service(store, state_store, client) + + with pytest.raises(KeboolaApiError) as exc_info: + service.login_password(stack=STACK_URL, email="svc@example.com", password="s3cr3t") + assert exc_info.value.error_code == ErrorCode.AUTH_MFA_INVALID diff --git a/tests/test_auth_totp.py b/tests/test_auth_totp.py new file mode 100644 index 00000000..b2f99a7b --- /dev/null +++ b/tests/test_auth_totp.py @@ -0,0 +1,45 @@ +"""Tests for auth/totp.py: RFC 6238 TOTP code computation.""" + +from __future__ import annotations + +from unittest.mock import patch + +from keboola_agent_cli.auth.totp import compute_totp_code + + +class TestComputeTotpCode: + def test_rfc6238_test_vector(self) -> None: + """RFC 6238 appendix B: seed 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ' (the base32 + encoding of the ASCII string '12345678901234567890') at T=59s -> '94287082' + for SHA1/8-digit. This module hardcodes 6 digits, so check the low-order + 6 digits of the same well-known reference value instead.""" + with patch("keboola_agent_cli.auth.totp.time.time", return_value=59): + code = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ", digits=8) + assert code == "94287082" + + def test_default_is_six_digits(self) -> None: + with patch("keboola_agent_cli.auth.totp.time.time", return_value=59): + code = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + assert len(code) == 6 + assert code.isdigit() + + def test_same_time_window_is_deterministic(self) -> None: + with patch("keboola_agent_cli.auth.totp.time.time", return_value=1000): + first = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + second = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + assert first == second + + def test_different_time_window_usually_differs(self) -> None: + with patch("keboola_agent_cli.auth.totp.time.time", return_value=0): + first = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + with patch("keboola_agent_cli.auth.totp.time.time", return_value=10_000_000): + second = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + assert first != second + + def test_accepts_lowercase_and_whitespace(self) -> None: + with patch("keboola_agent_cli.auth.totp.time.time", return_value=59): + lower = compute_totp_code("gezdgnbvgy3tqojqgezdgnbvgy3tqojq") + spaced = compute_totp_code("GEZD GNBV GY3T QOJQ GEZD GNBV GY3T QOJQ") + plain = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + assert lower == plain + assert spaced == plain diff --git a/tests/test_cli_auth.py b/tests/test_cli_auth.py index b5e11aee..9948a3ab 100644 --- a/tests/test_cli_auth.py +++ b/tests/test_cli_auth.py @@ -214,6 +214,101 @@ def test_state_mismatch_exit_code(self, tmp_path: Path) -> None: assert json.loads(result.output)["error"]["code"] == "AUTH_STATE_MISMATCH" +class TestLoginPassword: + def test_success_forwards_args_and_computed_totp_code(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.return_value = _login_result(method="password") + result = _invoke( + config_dir, + svc, + [ + "auth", + "login-password", + "--email", + "svc@example.com", + "--password", + "s3cr3t", + "--totp-secret", + "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ", + ], + ) + assert result.exit_code == 0, result.output + kwargs = svc.login_password.call_args.kwargs + assert kwargs["email"] == "svc@example.com" + assert kwargs["password"] == "s3cr3t" + assert kwargs["totp_code"] is not None + assert kwargs["totp_code"].isdigit() + assert len(kwargs["totp_code"]) == 6 + + def test_no_totp_secret_passes_none(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.return_value = _login_result(method="password") + result = _invoke( + config_dir, + svc, + ["auth", "login-password", "--email", "svc@example.com", "--password", "s3cr3t"], + ) + assert result.exit_code == 0, result.output + assert svc.login_password.call_args.kwargs["totp_code"] is None + + def test_env_vars_populate_email_and_password(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.return_value = _login_result(method="password") + with ( + patch("keboola_agent_cli.cli.AuthService", return_value=svc), + patch.dict( + "os.environ", + {"KBC_LOGIN_EMAIL": "svc@example.com", "KBC_LOGIN_PASSWORD": "s3cr3t"}, + ), + ): + result = runner.invoke(app, ["--config-dir", str(config_dir), "auth", "login-password"]) + assert result.exit_code == 0, result.output + kwargs = svc.login_password.call_args.kwargs + assert kwargs["email"] == "svc@example.com" + assert kwargs["password"] == "s3cr3t" + + def test_mfa_invalid_error_surfaces(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.side_effect = KeboolaApiError( + "webauthn-only account", error_code=ErrorCode.AUTH_MFA_INVALID + ) + result = _invoke( + config_dir, + svc, + ["--json", "auth", "login-password", "--email", "e", "--password", "p"], + ) + assert result.exit_code != 0 + assert json.loads(result.stdout)["error"]["code"] == "AUTH_MFA_INVALID" + + def test_password_never_appears_in_output(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.return_value = _login_result(method="password") + result = _invoke( + config_dir, + svc, + [ + "--json", + "auth", + "login-password", + "--email", + "svc@example.com", + "--password", + "SuperSecretPassword123", + ], + ) + assert "SuperSecretPassword123" not in result.output + + class TestPostLoginHook: """The optional "register these projects now?" nudge after a plain login.""" diff --git a/uv.lock b/uv.lock index bd7a4c0e..10228f83 100644 --- a/uv.lock +++ b/uv.lock @@ -590,7 +590,7 @@ wheels = [ [[package]] name = "keboola-cli" -version = "0.83.0" +version = "0.84.0" source = { editable = "." } dependencies = [ { name = "croniter" }, From 55f21e63ffe428c7481adc337dddacb39a218a01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 10 Aug 2026 12:35:35 +0200 Subject: [PATCH 02/13] fix(auth): map malformed --totp-secret to ConfigError instead of a raw traceback Review found compute_totp_code() ran outside the command's try/except, so a non-base32 or blank TOTP secret crashed with an uncaught binascii.Error instead of the structured error every other input parser in this codebase produces. totp.py now validates and raises ValueError on empty/malformed input; the CLI maps that to ConfigError. Also adds TOTP period-boundary rotation and malformed/empty-secret test coverage. Co-Authored-By: Claude Sonnet 5 --- src/keboola_agent_cli/auth/totp.py | 15 +++++++++++++-- src/keboola_agent_cli/commands/auth.py | 5 ++++- tests/test_auth_totp.py | 21 +++++++++++++++++++++ tests/test_cli_auth.py | 23 +++++++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/keboola_agent_cli/auth/totp.py b/src/keboola_agent_cli/auth/totp.py index fd353cd2..fbb82ec9 100644 --- a/src/keboola_agent_cli/auth/totp.py +++ b/src/keboola_agent_cli/auth/totp.py @@ -10,6 +10,7 @@ from __future__ import annotations import base64 +import binascii import hashlib import hmac import struct @@ -17,8 +18,18 @@ def compute_totp_code(secret_b32: str, *, digits: int = 6, period: int = 30) -> str: - """Compute the current TOTP code for a base32-encoded secret (RFC 6238).""" - key = base64.b32decode(secret_b32.strip().upper().replace(" ", ""), casefold=True) + """Compute the current TOTP code for a base32-encoded secret (RFC 6238). + + Raises ``ValueError`` on a blank or malformed secret -- callers should map + that to a structured CLI error rather than letting it propagate raw. + """ + cleaned = secret_b32.strip().upper().replace(" ", "") + if not cleaned: + raise ValueError("TOTP secret is empty") + try: + key = base64.b32decode(cleaned, casefold=True) + except binascii.Error as exc: + raise ValueError(f"TOTP secret is not valid base32: {exc}") from exc counter = int(time.time() // period) digest = hmac.new(key, struct.pack(">Q", counter), hashlib.sha1).digest() offset = digest[-1] & 0x0F diff --git a/src/keboola_agent_cli/commands/auth.py b/src/keboola_agent_cli/commands/auth.py index de38a3b3..121769b1 100644 --- a/src/keboola_agent_cli/commands/auth.py +++ b/src/keboola_agent_cli/commands/auth.py @@ -443,13 +443,16 @@ def auth_login_password( formatter = get_formatter(ctx) service: AuthService = get_service(ctx, "auth_service") try: + totp_code = compute_totp_code(totp_secret) if totp_secret else None result = service.login_password( stack=stack, email=email, password=password, - totp_code=compute_totp_code(totp_secret) if totp_secret else None, + totp_code=totp_code, register_projects=register_projects, ) + except ValueError as exc: + _handle_errors(formatter, ConfigError(f"--totp-secret: {exc}")) except (ConfigError, KeboolaApiError) as exc: _handle_errors(formatter, exc) formatter.output(result, _format_login_result) diff --git a/tests/test_auth_totp.py b/tests/test_auth_totp.py index b2f99a7b..5df2e574 100644 --- a/tests/test_auth_totp.py +++ b/tests/test_auth_totp.py @@ -4,6 +4,8 @@ from unittest.mock import patch +import pytest + from keboola_agent_cli.auth.totp import compute_totp_code @@ -43,3 +45,22 @@ def test_accepts_lowercase_and_whitespace(self) -> None: plain = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") assert lower == plain assert spaced == plain + + def test_code_rotates_across_period_boundary(self) -> None: + with patch("keboola_agent_cli.auth.totp.time.time", return_value=29): + before = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + with patch("keboola_agent_cli.auth.totp.time.time", return_value=30): + after = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") + assert before != after + + def test_empty_secret_raises_value_error(self) -> None: + with pytest.raises(ValueError, match="empty"): + compute_totp_code("") + + def test_whitespace_only_secret_raises_value_error(self) -> None: + with pytest.raises(ValueError, match="empty"): + compute_totp_code(" ") + + def test_malformed_base32_raises_value_error(self) -> None: + with pytest.raises(ValueError, match="base32"): + compute_totp_code("not-valid-base32!!") diff --git a/tests/test_cli_auth.py b/tests/test_cli_auth.py index 9948a3ab..b2dcaf7d 100644 --- a/tests/test_cli_auth.py +++ b/tests/test_cli_auth.py @@ -288,6 +288,29 @@ def test_mfa_invalid_error_surfaces(self, tmp_path: Path) -> None: assert result.exit_code != 0 assert json.loads(result.stdout)["error"]["code"] == "AUTH_MFA_INVALID" + def test_malformed_totp_secret_raises_config_error_not_traceback(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + result = _invoke( + config_dir, + svc, + [ + "--json", + "auth", + "login-password", + "--email", + "e", + "--password", + "p", + "--totp-secret", + "not-valid-base32!!", + ], + ) + assert result.exit_code != 0 + assert json.loads(result.stdout)["error"]["code"] == "CONFIG_ERROR" + svc.login_password.assert_not_called() + def test_password_never_appears_in_output(self, tmp_path: Path) -> None: config_dir = tmp_path / "c" config_dir.mkdir() From 04cb5c5224c5f92719739446df9935229d224205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 10 Aug 2026 13:34:02 +0200 Subject: [PATCH 03/13] fix(auth): address Copilot review comments on login-password - Fixed a stale test docstring claiming "this module hardcodes 6 digits" after the RFC 6238 test already exercised digits=8. - Split the auth_client.py "PKCE" section header: login_password/ verify_mfa_totp are a distinct password-grant+MFA section, not PKCE. - Fixed a ConfigError message that told a CLI user to "pass totp_code" -- the public flag is --totp-secret; the code is computed internally. - Added --password-stdin (mirrors dev-portal identity add's existing pattern): --password is now optional, and login-password fails with a ConfigError (not a Typer usage error) if neither --password, --password-stdin, nor KBC_LOGIN_PASSWORD supplied a value. Regenerated SKILL.md's auto-generated decision table to match the now-optional flag. Added test coverage for --password-stdin (TTY and pipe paths) and the missing-password error path. Co-Authored-By: Claude Sonnet 5 --- plugins/kbagent/skills/kbagent/SKILL.md | 2 +- src/keboola_agent_cli/auth/auth_client.py | 6 +++- src/keboola_agent_cli/commands/auth.py | 35 ++++++++++++++++--- .../services/auth_service.py | 3 +- tests/test_auth_totp.py | 5 +-- tests/test_cli_auth.py | 27 ++++++++++++++ 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 8cb9d904..1363289a 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -69,7 +69,7 @@ When working inside a git repository or project directory, run `kbagent init` (o | Remove all permission restrictions | `kbagent permissions reset` | | Check if a specific operation is allowed | `kbagent permissions check ` | | Sign in to a Keboola stack via browser login (PKCE) or device code | `kbagent auth login` | -| Sign in via email + password (+ TOTP if the account has MFA) -- no browser | `kbagent auth login-password --email EMAIL --password PASSWORD` | +| Sign in via email + password (+ TOTP if the account has MFA) -- no browser | `kbagent auth login-password --email EMAIL` | | Show the programmatic-auth session health for a stack | `kbagent auth status` | | Revoke and clear the local programmatic-auth session for a stack | `kbagent auth logout` | | Register accessible projects from the current session as local aliases | `kbagent auth register-projects` | diff --git a/src/keboola_agent_cli/auth/auth_client.py b/src/keboola_agent_cli/auth/auth_client.py index 0dd87a61..6284dd0d 100644 --- a/src/keboola_agent_cli/auth/auth_client.py +++ b/src/keboola_agent_cli/auth/auth_client.py @@ -258,7 +258,7 @@ def __exit__(self, *args: Any) -> None: self.close() # ------------------------------------------------------------------ - # PKCE + # Password grant + MFA (unattended, CI-safe login) # ------------------------------------------------------------------ def login_password(self, email: str, password: str) -> CliTokenResponse | MfaChallengeResult: @@ -288,6 +288,10 @@ def verify_mfa_totp(self, mfa_token: str, code: str) -> CliTokenResponse: ) return CliTokenResponse.model_validate(response.json()) + # ------------------------------------------------------------------ + # PKCE + # ------------------------------------------------------------------ + def authorize_url(self, *, redirect_uri: str, code_challenge: str, state: str) -> str: """Build the browser-facing PKCE authorize URL. diff --git a/src/keboola_agent_cli/commands/auth.py b/src/keboola_agent_cli/commands/auth.py index 121769b1..613c9eb0 100644 --- a/src/keboola_agent_cli/commands/auth.py +++ b/src/keboola_agent_cli/commands/auth.py @@ -18,6 +18,7 @@ from __future__ import annotations +import getpass import sys from collections.abc import Mapping, Sequence from typing import Any, NoReturn @@ -385,6 +386,19 @@ def _run_post_login_hook( ) +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() + + @auth_app.command("login-password") def auth_login_password( ctx: typer.Context, @@ -394,13 +408,20 @@ def auth_login_password( envvar=ENV_KBC_LOGIN_EMAIL, help="Account email. Also settable via KBC_LOGIN_EMAIL.", ), - password: str = typer.Option( - ..., + password: str | None = typer.Option( + None, "--password", envvar=ENV_KBC_LOGIN_PASSWORD, help="Account password. Prefer KBC_LOGIN_PASSWORD (a CI secret in the step's " - "env: block) over typing this flag directly -- it avoids the value landing in " - "shell history or a process listing.", + "env: block) or --password-stdin over typing this flag directly -- it avoids " + "the value landing in shell history or a process listing.", + ), + password_stdin: bool = typer.Option( + False, + "--password-stdin", + help="Read the password from stdin instead of --password/KBC_LOGIN_PASSWORD. " + "On a TTY this is a hidden prompt (Enter to confirm); on a pipe it reads until " + 'EOF (e.g. `echo "$PASS" | kbagent auth login-password --password-stdin ...`).', ), totp_secret: str | None = typer.Option( None, @@ -442,6 +463,12 @@ def auth_login_password( """ formatter = get_formatter(ctx) service: AuthService = get_service(ctx, "auth_service") + if password_stdin: + password = _read_password_stdin() + if not password: + _handle_errors( + formatter, ConfigError("Pass --password, --password-stdin, or set KBC_LOGIN_PASSWORD.") + ) try: totp_code = compute_totp_code(totp_secret) if totp_secret else None result = service.login_password( diff --git a/src/keboola_agent_cli/services/auth_service.py b/src/keboola_agent_cli/services/auth_service.py index 0841bcbe..d45eae34 100644 --- a/src/keboola_agent_cli/services/auth_service.py +++ b/src/keboola_agent_cli/services/auth_service.py @@ -317,7 +317,8 @@ def login_password( ) if not totp_code: raise ConfigError( - "This account requires a TOTP code to sign in -- pass totp_code." + "This account requires a TOTP code to sign in -- pass " + "--totp-secret (kbagent computes the code from it)." ) tokens = client.verify_mfa_totp(result.mfa_token, totp_code) else: diff --git a/tests/test_auth_totp.py b/tests/test_auth_totp.py index 5df2e574..000a8bfe 100644 --- a/tests/test_auth_totp.py +++ b/tests/test_auth_totp.py @@ -13,8 +13,9 @@ class TestComputeTotpCode: def test_rfc6238_test_vector(self) -> None: """RFC 6238 appendix B: seed 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ' (the base32 encoding of the ASCII string '12345678901234567890') at T=59s -> '94287082' - for SHA1/8-digit. This module hardcodes 6 digits, so check the low-order - 6 digits of the same well-known reference value instead.""" + for SHA1/8-digit. ``digits`` defaults to 6 (the login-password CLI never + overrides it), but the RFC's published vector is 8-digit, so pass digits=8 + here to check against the exact reference value rather than a truncation.""" with patch("keboola_agent_cli.auth.totp.time.time", return_value=59): code = compute_totp_code("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ", digits=8) assert code == "94287082" diff --git a/tests/test_cli_auth.py b/tests/test_cli_auth.py index b2dcaf7d..c3416591 100644 --- a/tests/test_cli_auth.py +++ b/tests/test_cli_auth.py @@ -255,6 +255,33 @@ def test_no_totp_secret_passes_none(self, tmp_path: Path) -> None: assert result.exit_code == 0, result.output assert svc.login_password.call_args.kwargs["totp_code"] is None + def test_password_stdin_reads_piped_password(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + svc.login_password.return_value = _login_result(method="password") + result = _invoke( + config_dir, + svc, + ["auth", "login-password", "--email", "svc@example.com", "--password-stdin"], + input_text="s3cr3t\n", + ) + assert result.exit_code == 0, result.output + assert svc.login_password.call_args.kwargs["password"] == "s3cr3t" + + def test_missing_password_is_config_error(self, tmp_path: Path) -> None: + config_dir = tmp_path / "c" + config_dir.mkdir() + svc = MagicMock() + result = _invoke( + config_dir, + svc, + ["--json", "auth", "login-password", "--email", "svc@example.com"], + ) + assert result.exit_code != 0 + assert json.loads(result.stdout)["error"]["code"] == "CONFIG_ERROR" + svc.login_password.assert_not_called() + def test_env_vars_populate_email_and_password(self, tmp_path: Path) -> None: config_dir = tmp_path / "c" config_dir.mkdir() From 107ceab94e2b7c4c86109cb12a1a940013d21c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Tue, 11 Aug 2026 14:05:07 +0200 Subject: [PATCH 04/13] fix(auth): map login-password failures to specific codes, fix TOTP timing/replay Wrong-password/MFA failures on `login-password` no longer surface the generic session-oriented "Invalid or expired token" wording -- 401/403/404/429 on /v1/auth/login and /v1/auth/mfa now get a dedicated mapping naming the actual cause (AUTH_FLOW_DENIED, SSO/admin-MFA 403, rate-limit reset time). The TOTP seed is now passed into AuthService.login_password and the code computed immediately before the MFA request rather than before the login round trip, closing the timing gap a retried login could open. verify_mfa_totp now bypasses the shared retry loop (same exception poll_device_token/refresh already make) so a 429/5xx retry can never resubmit an already-consumed TOTP slice. totp.py also pads/strips seeds the way real enrollment UIs hand them out. --- src/keboola_agent_cli/auth/auth_client.py | 144 ++++++++++++++++-- src/keboola_agent_cli/auth/models.py | 5 + src/keboola_agent_cli/auth/totp.py | 6 +- src/keboola_agent_cli/commands/auth.py | 6 +- .../services/auth_service.py | 25 ++- tests/test_auth_client.py | 103 +++++++++++++ tests/test_auth_service.py | 35 ++++- tests/test_cli_auth.py | 20 ++- 8 files changed, 308 insertions(+), 36 deletions(-) diff --git a/src/keboola_agent_cli/auth/auth_client.py b/src/keboola_agent_cli/auth/auth_client.py index 6284dd0d..543d691f 100644 --- a/src/keboola_agent_cli/auth/auth_client.py +++ b/src/keboola_agent_cli/auth/auth_client.py @@ -10,10 +10,13 @@ token to revoke in its request body. Inherits shared retry/backoff (429/5xx) and error-mapping infrastructure from -:class:`BaseHttpClient`, with two deliberate exceptions that keep the mapping +:class:`BaseHttpClient`, with three deliberate exceptions that keep the mapping but skip the retry loop: `poll_device_token` (a polling 400 is a protocol -state, not a failure) and `refresh` (a blind retry would re-present the refresh -token, and it runs under a wall-clock ceiling the retry loop would outlast). +state, not a failure), `refresh` (a blind retry would re-present the refresh +token, and it runs under a wall-clock ceiling the retry loop would outlast), +and `verify_mfa_totp` (the server records each TOTP time-slice as consumed on +first submission, so a retry would replay a code guaranteed to be rejected -- +burning one of the account's limited MFA attempts for nothing). `refresh` makes one narrow exception of its own for a rotation deadlock, which the server reports in a form that proves nothing rotated. See their docstrings. """ @@ -280,12 +283,23 @@ def login_password(self, email: str, password: str) -> CliTokenResponse | MfaCha return CliTokenResponse.model_validate(data) def verify_mfa_totp(self, mfa_token: str, code: str) -> CliTokenResponse: - """Resolve a password-login MFA challenge via TOTP (``POST /v1/auth/mfa``).""" - response = self._do_request( + """Resolve a password-login MFA challenge via TOTP (``POST /v1/auth/mfa``). + + Bypasses `_do_request`/the shared retry loop -- the same deliberate + exception `poll_device_token` and `refresh` already make (see the + module docstring). The server consumes a TOTP time-slice on its + first submission and rejects any resubmission of it, so a 429/5xx + retry here would resend the same code and be rejected by + construction, burning one of the account's limited MFA attempts for + a failure that was never the credential's fault. + """ + response = self._client.request( "POST", AUTH_MFA_PATH, json={"mfaToken": mfa_token, "type": "totp", "code": code}, ) + if response.status_code >= 400: + self._map_auth_error(response) return CliTokenResponse.model_validate(response.json()) # ------------------------------------------------------------------ @@ -728,16 +742,28 @@ def _raise_api_error(self, response: httpx.Response, base_url: str | None = None def _map_auth_error(self, response: httpx.Response) -> NoReturn: """Map a failed auth-endpoint response, escalating 404 to a dedicated code. - A 404 here means programmatic auth (or the specific flow) is not - enabled on this stack -- a fail-closed feature flag -- not "wrong - URL". Surfacing the generic `NOT_FOUND` code would send the user - chasing a routing bug that does not exist; this maps it to - `AUTH_NOT_SUPPORTED_ON_STACK` with a message naming the static-token - fallback instead. Every other status delegates to the shared - `BaseHttpClient` mapping -- no retry loop is added around the 404 - case, since a disabled feature flag will not become enabled by + `/v1/auth/login` and `/v1/auth/mfa` get their own mapping + (`_raise_password_login_error`) before the generic rules below: both + are unauthenticated-by-definition endpoints where a 401 means "wrong + credential", not "stale token", so the inherited `INVALID_TOKEN` + wording (and its `mask_token("")` interpolation) would misdiagnose + the command's single most likely failure. See PR #565 review. + + For every other auth endpoint, a 404 means programmatic auth (or the + specific flow) is not enabled on this stack -- a fail-closed feature + flag -- not "wrong URL". Surfacing the generic `NOT_FOUND` code would + send the user chasing a routing bug that does not exist; this maps it + to `AUTH_NOT_SUPPORTED_ON_STACK` with a message naming the + static-token fallback instead. Every other status delegates to the + shared `BaseHttpClient` mapping -- no retry loop is added around the + 404 case, since a disabled feature flag will not become enabled by retrying. """ + if response.request is not None and response.request.url.path in ( + AUTH_LOGIN_PATH, + AUTH_MFA_PATH, + ): + self._raise_password_login_error(response) if response.status_code == 404: raise KeboolaApiError( message=( @@ -755,6 +781,98 @@ def _map_auth_error(self, response: httpx.Response) -> NoReturn: # divergence explicit rather than relax this method's NoReturn type. raise AssertionError("unreachable: BaseHttpClient._raise_api_error always raises") + def _raise_password_login_error(self, response: httpx.Response) -> NoReturn: + """Map a failed `/v1/auth/login` or `/v1/auth/mfa` response. + + These two endpoints are the password-grant login path + (`login_password` / `verify_mfa_totp`) and see a different failure + surface than every other auth endpoint: there is no client-level + credential to be "invalid or expired" here, only a submitted email, + password, or TOTP code that the server rejected outright. + + - 401 -- wrong email/password on `/v1/auth/login`, or a wrong/expired + TOTP code on `/v1/auth/mfa`; the message is picked by which path + failed. Mapped to `AUTH_FLOW_DENIED` (already used for a denied + PKCE/device flow), not the inherited "Invalid or expired token" + wording -- there is no client-level token to be stale here. + - 403 -- the server's own message is surfaced verbatim and + `kbagent auth login` is named, because the two real causes here + (an SSO-enforced account, or a super-admin account whose MFA the + password grant cannot resolve) both require the browser flow, not + a retry of this one. + - 404 -- unlike every other auth endpoint this is *not* browser + login, so the message says "programmatic auth" rather than + "Browser login". + - 429 -- `/v1/auth/login` rate-limits by email and by IP over a + 15-minute window and reports the tightest bucket via + `X-RateLimit-Reset` on every response from these two endpoints + specifically so a client can self-throttle. Surfacing it turns a + generic `API error 429` into "try again at