diff --git a/CLAUDE.md b/CLAUDE.md index e4615ed5..e7e9c33d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -554,7 +554,12 @@ kbagent feature user-show --project ALIAS --email EMAIL kbagent feature user-add --project ALIAS --email EMAIL --feature NAME [--dry-run] [--yes] kbagent feature user-remove --project ALIAS --email EMAIL --feature NAME [--dry-run] [--yes] -# token: scoped Storage tokens (Keboola single-bucket-write pattern; acting token needs canManageTokens; secret shown once). +# token: scoped Storage tokens (Keboola single-bucket-write pattern; secret shown once). +# create REQUIRES a MASTER (admin) token -- pre-flight MISSING_MASTER_TOKEN guard (exit 3) +# since #599: a non-master token with canManageTokens is CreateTokenVoter's "impossible state" +# and the API answers a generic 500. That defect is CREATE-ONLY: list/delete/refresh need only +# canManageTokens and are deliberately NOT guarded (RefreshTokenVoter also lets any token +# rotate itself), so rotating a leaked device token from an org-setup token keeps working. kbagent token list --project NAME [--with-last-used] [--columns NAME ...] kbagent token create --project NAME --description DESC [--bucket-write BUCKET ...] [--bucket-read BUCKET ...] [--component-access ID ...] [--can-read-all-file-uploads] [--expires-in N] kbagent token delete --project NAME --token-id ID [--yes] diff --git a/docs/sdk.md b/docs/sdk.md index dc8dd417..87293993 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -177,11 +177,11 @@ Seven methods for the "provision an OTLP ingest endpoint, then mint a narrowly-s **Scoped Storage tokens** — mint, list, revoke, rotate: -- **`create_scoped_token(*, description, bucket_permissions=None, component_access=None, can_read_all_file_uploads=False, expires_in=None) -> ScopedTokenResult`** — `POST /v2/storage/tokens`. `bucket_permissions` is `{bucket_id: "read"|"write"}`; `expires_in` is seconds. The acting token must carry **`canManageTokens`** or the create 403s. +- **`create_scoped_token(*, description, bucket_permissions=None, component_access=None, can_read_all_file_uploads=False, expires_in=None) -> ScopedTokenResult`** — `POST /v2/storage/tokens`. `bucket_permissions` is `{bucket_id: "read"|"write"}`; `expires_in` is seconds. The acting token must be a **master (admin) token**: a non-master token carrying `canManageTokens` gets a generic 500 (`CreateTokenVoter` `LogicException`, issue #599), one without the flag gets a 403. The SDK facade has no pre-flight guard (only the CLI/service layer does), so you hit the raw API behavior. - **`list_tokens() -> list[TokenListEntryResult]`** — `GET /v2/storage/tokens` (`0.86.0+`). Where the `token_id` for `delete_token` / `refresh_token` comes from. **Secrets are stripped before validation**: a project carrying the `force-decrypted-token` feature has the API embed live values in the listing, and `create_scoped_token` is meant to be the only reveal. The acting token needs `canManageTokens`. - **`list_tokens(*, with_last_used=False)`** — `with_last_used` (`0.88.0+`) additionally derives each token's most recent activity from `GET /v2/storage/tokens/{id}/events`, populating `last_used` / `last_used_event` / `last_used_status` and returning the entries dormant-first. **Opt-in: one extra request per token**, run in parallel. Read `last_used_status` rather than a bare `last_used`: `used` (timestamp is real), `never` (**proven** unused — minted inside the ~6-month event-retention window with no activity), `unknown` (older than retention, so the API cannot say — do *not* treat as never), `error` (that token's lookup failed; the entry degrades, the call still returns). Activity inside a **development branch is invisible** to this feed, so a branch-only token reads as dormant. - **`delete_token(token_id) -> None`** — `DELETE /v2/storage/tokens/{id}` (204, no body). Revokes. -- **`refresh_token(token_id) -> ScopedTokenResult`** — `POST .../tokens/{id}/refresh`. Rotates the secret in place; the returned `.token` is the new secret. +- **`refresh_token(token_id) -> ScopedTokenResult`** — `POST .../tokens/{id}/refresh`. Rotates the secret in place; the returned `.token` is the new secret. Any token may refresh itself; refreshing another token needs `canManageTokens`. No master token required — the `CreateTokenVoter` defect is create-only, so neither this facade nor the CLI guards it. ```python tok = kbc.create_scoped_token( @@ -202,7 +202,7 @@ device_secret = tok.token # ONE-TIME reveal — see the gotcha below **The two-call enrollment example** — provision the endpoint, then mint the device's token scoped to exactly the sink bucket: ```python -with Client(url=URL, token=TOKEN) as kbc: # TOKEN must have canManageTokens +with Client(url=URL, token=TOKEN) as kbc: # TOKEN must be a master token for create src = kbc.create_stream_source("my-source") # StreamSourceResult # hand the device its ingest endpoint: print(src.otlp_url) # carries the ingest secret in the path — UNMASKED diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 9631a14f..71acdc45 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -105,7 +105,7 @@ been retired, so its absence is NOT a promise (see §1 Rule 6). | Edit SQL transformation blocks/codes | `kbagent transformation show` (FRESH ids) then `kbagent transformation edit --config-id K --change-description T --op '{"op":"set_code",...}'` -- 9 ops, ids `b{i}` / `b{i}.c{j}`, `--storage` REPLACES wholesale | `kbagent config update --configuration @body.json` (auto-normalizes `script[]`) | `transformation edit` without a fresh `show` (positional ids renumber); raw `PUT` (skips `script[]` normalization) | | Run a job (and wait) | `kbagent job run --project P --component-id C --config-id K --wait` | -- | `job run` without `--wait` when the user expects the result | | Provision / read an OTLP Data Streams endpoint | `kbagent stream create-source -p P --name N --type otlp [--if-not-exists]` (auto-creates logs/metrics/traces sinks) then `stream detail N -p P --reveal` | `stream list`; `--no-sinks` for a bare source | deriving the `stream-in` URL yourself (use `source.otlp.url`); printing the secret unasked | -| Mint / rotate / revoke a scoped Storage token | `kbagent token create -p P -d DESC [--bucket-write B ...] [--expires-in N]` / `token refresh` / `token delete --token-id ID` -- acting token needs `canManageTokens`, secret shown ONCE (persist `id` + `expires` only); `token list` (0.86.0+) is the only source of an existing `--token-id` | same ops on the SDK facade: `Client.create_scoped_token / refresh_token / delete_token` | `--component-access` / `--can-read-all-file-uploads` for an UPLOAD token (uploads need `--bucket-write` on the sink bucket; those flags gate READING others' uploads); claiming `stream create-source` needs a master token | +| Mint / rotate / revoke a scoped Storage token | `kbagent token create -p P -d DESC [--bucket-write B ...] [--expires-in N]` / `token refresh` / `token delete --token-id ID` -- `create` needs a MASTER token (0.89.0+ pre-flight `MISSING_MASTER_TOKEN`; `canManageTokens` alone = server 500, #599), refresh/list/delete need only `canManageTokens`; secret shown ONCE (persist `id` + `expires` only); `token list` (0.86.0+) is the only source of an existing `--token-id` | same ops on the SDK facade: `Client.create_scoped_token / refresh_token / delete_token` | `--component-access` / `--can-read-all-file-uploads` for an UPLOAD token (uploads need `--bucket-write` on the sink bucket; those flags gate READING others' uploads); claiming `stream create-source` needs a master token | | Search items by name across projects | `kbagent search QUERY [--project P] [--type table\|bucket\|config\|flow\|data-app\|transformation] [--search-type textual\|config-based] [--regex]` -- `--regex` matches WHOLE terms on entity names: `report` does NOT match `monthly_report`, write `.*report.*` | -- | `--regex` with `--search-type config-based` (exit 2) | | Search config JSON bodies | `kbagent search QUERY --search-type config-based [--project P]` (case-insensitive) | `kbagent config search --query Q` (config bodies only, no tables/buckets; case-SENSITIVE unless `-i`) | pulling every config with `config detail` to grep locally | | Browse configs (exploration) | `kbagent config list` / `kbagent config search --query Q` | -- | a full-project pull just to grep locally | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 6200f2e5..9667eea2 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -202,12 +202,12 @@ Uses the per-project Storage token (no manage token). Control plane = `stream. 204). Destructive; confirms via prompt unless `--yes` or `--json`. -- `token refresh --project NAME --token-id ID [--yes]` -- rotate a token's secret (POST `.../refresh`); the new secret is printed ONCE. Confirms unless `--yes` or `--json`. +- `token refresh --project NAME --token-id ID [--yes]` -- rotate a token's secret (POST `.../refresh`); the new secret is printed ONCE. Confirms unless `--yes` or `--json`. Needs `canManageTokens`, NOT a master token (unlike `create`). The new secret is not written back to config.json -- rotating the alias's own token leaves it holding a dead value until `project edit --project ALIAS --token `. ## Data Lineage - `lineage build -d DIR -o FILE [--refresh] [--ai]` -- build column-level lineage graph from sync'd data diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index f69048a0..82b638b9 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -294,10 +294,26 @@ Versioning convention: --token-id ID` revokes. All three back the SDK facade (`Client.create_scoped_token` / `refresh_token` / `delete_token`) and the same service layer. -- **The acting token must carry `canManageTokens`.** Without it the create/refresh - `POST /v2/storage/tokens[...]` returns 403. A normal project-admin storage token - has it; a narrowly-scoped device token does NOT — you cannot bootstrap tokens - from a token you just minted unless you granted it `canManageTokens`. +- **`create` requires a MASTER (admin) token — `canManageTokens` alone is NOT + enough.** A non-master token carrying the flag is the Storage API's + "impossible state" (`CreateTokenVoter` `LogicException` → generic 500, issue + #599) — exactly the shape `org setup` / `project refresh` mint. Since v0.89.0 + `token create` pre-flights `isMasterToken` and fails fast with + `MISSING_MASTER_TOKEN` (exit 3) naming the fix (`kbagent project edit + --project ALIAS --token `). +- **That defect is CREATE-ONLY — `refresh` / `list` / `delete` are NOT + master-guarded.** `RefreshTokenVoter` lets any token rotate **itself** and a + `canManageTokens` token rotate another, so rotating a leaked device token + from an `org setup` project token works and kbagent deliberately does not + block it (the incident path is the last place to add a credential upgrade + step). Do not "fix" a failing `refresh` by hunting for a master token — read + the actual error. A narrowly-scoped device token has neither privilege, so + you still cannot bootstrap tokens from a token you just minted. +- **`token refresh` does not write the new secret back to `config.json`.** The + value is printed once, exactly like `create`. Rotating the very token an + alias uses therefore leaves that alias holding a dead value — follow it with + `kbagent project edit --project ALIAS --token ` or the next command + fails on auth. - **The secret is a ONE-TIME reveal.** `create` / `refresh` print the token value once (human mode: inside a Rich panel; `--json`: the `token` field). It is never retrievable again — persist only the `id` (to revoke/refresh later) and `expires`. @@ -2972,7 +2988,8 @@ project list should do the same — never render the bare null. The OAuth wizard URL embeds a short-lived **child** Storage API token scoped to the target component. Minting this child token via `POST /v2/storage/tokens` -requires `canManageTokens` privilege, which only **master tokens** carry. +requires a **master (admin) token** — `canManageTokens` alone is not enough +(issue #599: a non-master token carrying it makes the API 500 instead of 403). - Pre-flight: `kbagent` calls `verify_token` first and refuses with `MISSING_MASTER_TOKEN` (exit 3) before any HTTP write happens. Without this @@ -2983,10 +3000,12 @@ requires `canManageTokens` privilege, which only **master tokens** carry. the OAuth flow via the Keboola UI instead. - AI agents creating the project token via `kbagent project add` / `kbagent project refresh` get a non-master token by default — they must - switch to a master token before calling `config oauth-url`. See - https://github.com/keboola/cli/issues/ for the upstream - request to make `project add` / `project refresh` mint a token with - `canManageTokens` so OAuth flows work out of the box. + switch to a master token before calling `config oauth-url`. Granting the + minted token `canManageTokens` would NOT help: issue #599 established the + Storage API rejects a non-admin token carrying that flag with a 500 + (`CreateTokenVoter` "impossible state"), so a master token is the only + working credential here. `token create` carries the same guard since + v0.89.0 (`token refresh` does not — that endpoint has no such defect). ## `data-app logs` is the only unconstrained log surface (since v0.43.8) @@ -4057,8 +4076,9 @@ the web UI. `token refresh` are the only reveals, and a listing that dumped live values would break that contract for every token in the project at once. Do not reach for `kbagent http get` to work around this. -- **It needs `canManageTokens`, same as `create`.** A plain Storage token gets - a 403 -> `ACCESS_DENIED`. +- **It needs `canManageTokens` — but unlike `create` it does NOT need a master + token** (since v0.89.0 the mint is master-guarded, issue #599). A plain + Storage token gets a 403 -> `ACCESS_DENIED`. - The master token appears in the listing with `isMasterToken: true` and cannot be deleted -- the API refuses. - SDK parity: `Client.list_tokens() -> list[TokenListEntryResult]`, secrets diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index dda183fc..b4d6b350 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -47,6 +47,25 @@ "Plugin docs: `CLAUDE.md`'s command list had never included `config delete` at all. " "That silent drift made the command look nonexistent to AI agents reading it. " "Added alongside the new commands, with the double-delete trap recorded in gotchas.md.", + "Fix (#599): `token create` on a non-master token now fails fast with " + "`MISSING_MASTER_TOKEN` (exit 3) instead of the API's generic 500 error. " + "Root cause found in #599: the API's `CreateTokenVoter` " + "treats a non-admin token carrying `canManageTokens` as an impossible state " + "and throws a `LogicException` -- and that is exactly the token shape " + "`org setup` / `project refresh` mint, so every project onboarded that way " + "hit the 500. The new pre-flight guard mirrors the existing `config oauth-url` " + "one (`isMasterToken` via `GET /v2/storage/tokens/verify`, checked before any " + "write) and its message names the fix: point the alias at a master token via " + "`kbagent project edit --project ALIAS --token `. The defect is " + "create-only, so `token refresh` / `token list` / `token delete` are " + "unchanged and deliberately NOT guarded: `RefreshTokenVoter` lets any token " + "rotate itself and a `canManageTokens` token rotate another, and guarding " + "that would break a working incident path -- rotating a leaked device token " + "from an `org setup` project token -- to fix nothing.", + "Change (#599): token-group docs no longer claim `canManageTokens` is enough " + "for `token create` -- the real requirement there is a master token. " + "`canManageTokens` alone is necessary but not sufficient for the mint; it is " + "sufficient for `token list` / `token delete` / `token refresh`.", ], "0.88.0": [ "Fix (#624): column descriptions are now written where the Keboola UI and the " diff --git a/src/keboola_agent_cli/client/tokens.py b/src/keboola_agent_cli/client/tokens.py index 1b5c1416..9c354857 100644 --- a/src/keboola_agent_cli/client/tokens.py +++ b/src/keboola_agent_cli/client/tokens.py @@ -130,9 +130,15 @@ def create_scoped_token( ``can_read_all_file_uploads`` only widens *reading* files uploaded by *other* tokens (a device sees its own uploads regardless). - The acting token must carry ``canManageTokens`` (the API rejects the - create otherwise -- surfaced as an ``ACCESS_DENIED`` :class:`KeboolaApiError` - with the token masked). The returned dict is the raw API response; its + The acting token must be a **master (admin) token** -- ``canManageTokens`` + alone is not sufficient. ``POST /v2/storage/tokens`` authorizes via + ``CreateTokenVoter``, which throws a ``LogicException`` ("Normal token + cannot have manage tokens") for a non-admin token carrying that flag, + surfaced as a generic 500 "Application error." rather than a 403 + (issue #599; a token without the flag at all gets a clean 403 + ``ACCESS_DENIED``). ``TokenService`` pre-flights this via + :meth:`get_project_info` ``isMasterToken``; direct SDK callers hit the + raw API behavior. The returned dict is the raw API response; its ``token`` field is a **one-time** secret reveal -- persist only ``id`` (for :meth:`delete_token` / :meth:`refresh_token`) and ``expires``. @@ -168,7 +174,8 @@ def list_tokens(self) -> list[dict[str, Any]]: the token was minted by another token) ``creatorToken``. The acting token must carry ``canManageTokens``; the API answers 403 - otherwise (surfaced as ``ACCESS_DENIED``). + otherwise (surfaced as ``ACCESS_DENIED``). Unlike the create write it + does **not** need to be a master token. A **secret is never listed here as a rule, but the API is not a guarantee**: on a project carrying the ``force-decrypted-token`` @@ -253,6 +260,13 @@ def refresh_token(self, token_id: str) -> dict[str, Any]: **old** token string becomes immediately invalid (rotation, not additive), so every place using it must be updated. The token id is stable across a refresh. + + Server-side (``RefreshTokenVoter``) any token may refresh **itself** + and refreshing *another* token needs ``canManageTokens``. No master + token required -- the ``CreateTokenVoter`` defect behind + :meth:`create_scoped_token`'s master-token guard is create-only, so + ``TokenService.refresh_token`` deliberately does not guard this + (issue #599). """ response = self._request( "POST", f"/v2/storage/tokens/{quote(str(token_id), safe='')}/refresh" diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index b6199d52..42193cbd 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -827,7 +827,8 @@ kbagent token list --project NAME [--with-last-used] [--columns NAME ...] List the project's Storage API tokens (id, description, created, refreshed, expires, master flag, creating token). Secrets are never listed -- `token create` is the only reveal. This is - where the --token-id for delete/refresh comes from. Acting token needs canManageTokens. + where the --token-id for delete/refresh comes from. Acting token needs canManageTokens + (master NOT required -- unlike `token create`). --with-last-used (0.88.0+) answers "which of these are still in use": it derives each token's most recent activity from its own event feed and sorts dormant-first, so reading order is cleanup order. Opt-in -- it is ONE EXTRA API CALL PER TOKEN (fanned out in parallel). @@ -850,13 +851,19 @@ Create a scoped Storage API token (Keboola single-bucket-write pattern). --bucket-write / --bucket-read (repeatable) grant per-bucket write/read; write wins when a bucket is on both. --component-access (repeatable) restricts to named components. The token secret is printed ONCE - in a Rich Panel -- store it now, it is never retrievable again. Acting token needs canManageTokens. + in a Rich Panel -- store it now, it is never retrievable again. REQUIRES A MASTER (admin) + token -- canManageTokens alone gets MISSING_MASTER_TOKEN (pre-flight guard, exit 3; a + non-master token with that flag makes the API 500, issue #599). kbagent token delete --project NAME --token-id ID [--yes] Revoke a token by its numeric id (destructive; confirms unless --yes / --json). kbagent token refresh --project NAME --token-id ID [--yes] Rotate a token's secret (new secret printed ONCE; confirms unless --yes / --json). - Notes: uses the per-project Storage token (no manage token); the acting token must have the - canManageTokens privilege. The importable SDK (Client(url,token)) mirrors these as + Needs canManageTokens only -- NOT master-guarded (that API defect is create-only). The new + secret is NOT written back to config.json: rotating the alias's own token leaves it dead + until `project edit --project ALIAS --token `. + Notes: uses the per-project Storage token (no manage token). `token create` requires a MASTER + (admin) token; list/delete/refresh only need the canManageTokens privilege. The importable SDK + (Client(url,token)) has NO pre-flight guard and mirrors these as create_scoped_token / delete_token / refresh_token (dicts on .raw, typed ScopedTokenResult on the facade). ### Sharing (Cross-Project) diff --git a/src/keboola_agent_cli/commands/token.py b/src/keboola_agent_cli/commands/token.py index 77238b39..95e90e08 100644 --- a/src/keboola_agent_cli/commands/token.py +++ b/src/keboola_agent_cli/commands/token.py @@ -5,8 +5,11 @@ These are Storage API operations authenticated with the per-project Storage token kbagent already stores (``X-StorageApi-Token``) -- no manage token, no -extra prompt. The acting token must carry ``canManageTokens`` (the API rejects -the mint/rotate otherwise). +extra prompt. ``token create`` requires a **master (admin) Storage token**: +``canManageTokens`` alone is not enough there (the API answers a generic 500, +issue #599), so a pre-flight guard fails fast with ``MISSING_MASTER_TOKEN`` +instead. ``token list`` / ``token delete`` / ``token refresh`` need only +``canManageTokens`` and are deliberately unguarded. ``token create`` mints a scoped token and prints its secret value **once** -- kbagent never persists it. Store it immediately; it cannot be retrieved again. @@ -239,8 +242,10 @@ def token_create( Grants only what you pass: bucket read/write, component access, expiry. A token with just --bucket-write on one bucket can upload Files and write that - bucket, nothing else -- the Keboola single-bucket-write pattern. The acting - project token must carry canManageTokens. + bucket, nothing else -- the Keboola single-bucket-write pattern. Requires a + master (admin) project token -- canManageTokens alone is not enough (issue + #599); non-master tokens fail fast with MISSING_MASTER_TOKEN before any + write. """ formatter = get_formatter(ctx) service = get_service(ctx, "token_service") @@ -281,7 +286,8 @@ def token_list( Answers "what already exists" and hands you the token id that `token delete` and `token refresh` require, without a detour through the web UI. The acting - project token must carry canManageTokens, same as `token create`. + project token must carry canManageTokens -- unlike `token create` it does + NOT need to be a master token. --with-last-used answers the follow-up question -- which of them are still in use -- by deriving each token's most recent activity from its event @@ -356,7 +362,13 @@ def token_refresh( token_id: str = typer.Option(..., "--token-id", help="ID of the token to rotate"), yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation prompt"), ) -> None: - """Rotate a token: generate a new value and invalidate the old one (secret shown once).""" + """Rotate a token: generate a new value and invalidate the old one (secret shown once). + + Needs canManageTokens, NOT a master token -- unlike `token create` (the API + defect behind that guard is create-only, issue #599). Note the new secret is + printed but not stored: rotating the token this project alias itself uses + leaves the alias holding a dead value until you run `project edit --token`. + """ formatter = get_formatter(ctx) if ( not formatter.json_mode diff --git a/src/keboola_agent_cli/services/token_service.py b/src/keboola_agent_cli/services/token_service.py index 8a4e0adb..af596b99 100644 --- a/src/keboola_agent_cli/services/token_service.py +++ b/src/keboola_agent_cli/services/token_service.py @@ -6,9 +6,19 @@ :class:`KeboolaClient` factory (testability). These are **Storage API** operations authenticated with the per-project Storage -token (``X-StorageApi-Token``) -- no manage token is involved. The acting token -must itself carry ``canManageTokens``; the API rejects the mint/rotate otherwise -(surfaced as an ``ACCESS_DENIED`` :class:`KeboolaApiError` with the token masked). +token (``X-StorageApi-Token``) -- no manage token is involved. **Minting** +requires a **master (admin) Storage token** -- ``canManageTokens`` alone is not +sufficient: the Storage API's ``CreateTokenVoter`` treats a non-admin token +carrying that flag as an impossible state and throws a ``LogicException`` +(surfaced as a generic 500 "Application error."), which is exactly the shape of +token ``org setup`` / ``project refresh`` mint (issue #599). A pre-flight guard +turns that into a clean ``MISSING_MASTER_TOKEN`` error before any write. + +That defect is create-only. Rotating, listing and deleting need just +``canManageTokens`` (``RefreshTokenVoter`` additionally lets any token rotate +itself) and are deliberately **not** guarded -- blocking them would break +working paths, most sharply the incident one: rotating a leaked device token +from an ``org setup`` project token. The scoped-token use case is the Keboola "single-bucket write token" pattern (and device enrollment, ADR 0005 in keboola/jasnost): mint a narrow, expiring @@ -22,6 +32,7 @@ from typing import Any from ..client import KeboolaClient +from ..errors import ErrorCode, KeboolaApiError from ._token_last_used import dormancy_rank, enrich_tokens from .base import ( BaseService, @@ -83,6 +94,7 @@ def create_scoped_token( bucket_permissions[bucket_id] = "write" client = self._client_factory(creds.stack_url, creds.token) try: + self._require_master_token(client, alias=alias) result = client.create_scoped_token( description=description, bucket_permissions=bucket_permissions or None, @@ -112,7 +124,9 @@ def list_tokens(self, *, alias: str, with_last_used: bool = False) -> dict[str, that only wants an id to hand to ``token delete`` must not pay for it, and machine consumers keep the exact response shape they parse today. - The acting token needs ``canManageTokens``, same as create/refresh. + The acting token needs ``canManageTokens`` -- unlike ``create`` it does + **not** need to be a master token, so this listing is deliberately not + behind :meth:`_require_master_token`. """ creds = self._resolve_project(alias) client = self._client_factory(creds.stack_url, creds.token) @@ -152,7 +166,15 @@ def delete_token(self, *, alias: str, token_id: str) -> dict[str, Any]: client.close() def refresh_token(self, *, alias: str, token_id: str) -> dict[str, Any]: - """Rotate a token (old value invalidated) in ``alias``'s project.""" + """Rotate a token (old value invalidated) in ``alias``'s project. + + Deliberately **not** behind :meth:`_require_master_token`. The + ``CreateTokenVoter`` defect that guard exists for lives only on the + create endpoint; ``RefreshTokenVoter`` lets any token rotate itself and + a ``canManageTokens`` token rotate another, both of which work today. + Guarding here would block a working incident path -- revoking a leaked + device token from an ``org setup`` project token -- to fix nothing. + """ creds = self._resolve_project(alias) client = self._client_factory(creds.stack_url, creds.token) try: @@ -161,6 +183,44 @@ def refresh_token(self, *, alias: str, token_id: str) -> dict[str, Any]: finally: client.close() + def _require_master_token(self, client: KeboolaClient, *, alias: str) -> None: + """Fail fast unless the acting token is a master (admin) token. + + Pre-flight for the token mint only (``token refresh`` is deliberately + exempt -- see :meth:`refresh_token`), mirroring the + ``config oauth-url`` guard: ``POST /v2/storage/tokens`` authorizes via + ``CreateTokenVoter``, which throws a ``LogicException`` ("Normal token + cannot have manage tokens") for a non-admin token carrying + ``canManageTokens`` -- surfaced to the caller as a vague 500 + "Application error." instead of a 403. Every token minted by + ``org setup`` / ``project refresh`` is exactly that shape (issue #599), + so without this check the operator gets a misleading server-side error + for what is really a local credential problem. + + Raises: + KeboolaApiError: ``MISSING_MASTER_TOKEN`` (403) when the acting + token is not a master token. + """ + info = client.get_project_info() + if not info.get("isMasterToken", False): + raise KeboolaApiError( + status_code=403, + error_code=ErrorCode.MISSING_MASTER_TOKEN, + message=( + f"`token create` requires a master Storage API token on " + f"project '{alias}'. The current token " + f"(id={info.get('id', '?')}, " + f"description='{info.get('description', '?')}') is not a " + f"master token -- `canManageTokens` alone is not enough: " + f"the Storage API rejects the request with a generic 500 " + f"'Application error.' (CreateTokenVoter LogicException, " + f"issue #599). Point the alias at a master token " + f"(`kbagent project edit --project {alias} --token `) " + f"-- master = the token from your own user account in the " + f"Keboola UI, `isMasterToken: true` in `kbagent token list`." + ), + ) + def _resolve_project(self, alias: str) -> ResolvedProjectCredentials: """Resolve ``alias`` to its stack URL + token (or raise ConfigError).""" return resolve_project_credentials(self._config_store, alias) diff --git a/tests/test_token_service.py b/tests/test_token_service.py index 1f97c3d2..87a2ae76 100644 --- a/tests/test_token_service.py +++ b/tests/test_token_service.py @@ -4,6 +4,7 @@ from datetime import UTC, datetime, timedelta from pathlib import Path +from typing import ClassVar from unittest.mock import MagicMock import pytest @@ -37,6 +38,13 @@ def store(tmp_config_dir: Path) -> ConfigStore: @pytest.fixture def client_factory() -> tuple[MagicMock, MagicMock]: mock = MagicMock() + # Explicit master-token info so tests exercising `create` pass the + # pre-flight guard deliberately, not via a truthy MagicMock return. + mock.get_project_info.return_value = { + "id": "6610637", + "description": "master", + "isMasterToken": True, + } factory = MagicMock(return_value=mock) return factory, mock @@ -113,6 +121,79 @@ def test_unknown_alias_raises(self, store, client_factory) -> None: _svc(store, factory).create_scoped_token(alias="nope", description="x") +class TestMasterTokenGuard: + """`token create` pre-flight: the acting token must be a master token. + + The Storage API's `CreateTokenVoter` treats "a normal token carrying + `canManageTokens`" as an impossible state and throws a `LogicException`, + surfaced to the caller as a generic 500 "Application error." -- exactly the + shape of token `org setup` / `project refresh` mint (issue #599). The + guard turns that into a clean local MISSING_MASTER_TOKEN before any write, + mirroring the existing `config oauth-url` guard. It covers the mint ONLY -- + refresh/list/delete have no such defect and stay unguarded. + """ + + NON_MASTER_INFO: ClassVar[dict[str, object]] = { + "id": "7434918", + "description": "kbagent-cli [petr@keboola.com]", + "isMasterToken": False, + } + + def test_create_rejects_non_master_token_before_any_write(self, store, client_factory) -> None: + factory, mock = client_factory + mock.get_project_info.return_value = self.NON_MASTER_INFO + with pytest.raises(KeboolaApiError) as excinfo: + _svc(store, factory).create_scoped_token(alias=ALIAS, description="repro") + assert excinfo.value.error_code == ErrorCode.MISSING_MASTER_TOKEN + assert excinfo.value.status_code == 403 + # the guard must fire BEFORE the POST -- nothing may reach the API + mock.create_scoped_token.assert_not_called() + mock.close.assert_called_once() + + def test_create_error_names_the_remedy(self, store, client_factory) -> None: + """The message must carry the token identity and the `project edit` fix.""" + factory, mock = client_factory + mock.get_project_info.return_value = self.NON_MASTER_INFO + with pytest.raises(KeboolaApiError) as excinfo: + _svc(store, factory).create_scoped_token(alias=ALIAS, description="repro") + message = str(excinfo.value) + assert "7434918" in message + assert f"kbagent project edit --project {ALIAS}" in message + + def test_create_passes_on_master_token(self, store, client_factory) -> None: + factory, mock = client_factory + mock.create_scoped_token.return_value = {"id": "1", "token": "t"} + result = _svc(store, factory).create_scoped_token(alias=ALIAS, description="ok") + assert result["id"] == "1" + mock.get_project_info.assert_called_once() + mock.create_scoped_token.assert_called_once() + + def test_refresh_delete_and_list_are_not_guarded(self, store, client_factory) -> None: + """Only the mint is guarded -- the other three must keep working. + + The `CreateTokenVoter` defect is create-only. `RefreshTokenVoter` lets + any token rotate itself and a `canManageTokens` token rotate another, + and list/delete need the same flag (all verified in #599), so guarding + them would block working paths -- most sharply the incident one: + rotating a leaked device token from an `org setup` project token. + """ + factory, mock = client_factory + mock.get_project_info.return_value = self.NON_MASTER_INFO + mock.refresh_token.return_value = {"id": "999", "token": "999-new"} + mock.list_tokens.return_value = [] + svc = _svc(store, factory) + + assert svc.refresh_token(alias=ALIAS, token_id="999")["id"] == "999" + svc.delete_token(alias=ALIAS, token_id="1") + svc.list_tokens(alias=ALIAS) + + mock.refresh_token.assert_called_once_with("999") + mock.delete_token.assert_called_once() + mock.list_tokens.assert_called_once() + # not even the pre-flight verify goes out -- no wasted request either + mock.get_project_info.assert_not_called() + + class TestDeleteToken: def test_delete_shape(self, store, client_factory) -> None: factory, mock = client_factory