Repository navigation
feat: accept browser-login (session) tokens for the commands whose backend supports them (CLI-13) - #759
Conversation
The scheduler, data streams, data science, metastore and AI clients refused a kbc-session:// project and required a static Storage token, even though their backends already accept an OAuth bearer token. Route them through the session, the same as the Storage and Manage clients. services/base.py gains make_session_aware_client_factory(config_store, client_cls): it builds any BaseHttpClient subclass with BearerAuth for a kbc-session:// project and with a static token otherwise. make_client_factory now delegates to it. Each client below accepts http_auth and drops the X-StorageApi-Token header when it is set, the same as ManageClient, and each service builds it through the session-aware factory. A browser-login project now reaches: - flow schedule / flow schedule-remove (Scheduler) - stream (Data Streams) - data-app (Data Science) - semantic-layer (Metastore) - kai, docs query, config new / config examples, component detail, flow new / update / validate (AI service) The static-token guard stays as the fallback for the no-bearer path: a client built with a session token and no bearer still fails fast.
resolve_master_token fell back to require_static_token, which rejected a kbc-session:// project outright. It now returns the project's own token instead. The session-aware client factory sends that token as a bearer, and the Storage API enforces the master privilege. A master session passes. A non-master one gets its own 403. config oauth-url and data-app git-credentials already work for a session project. Both build a session-aware client and check the token's real isMasterToken privilege, not whether it is static or a session. Neither needs a change.
The scheduler, data streams, data science, metastore and AI clients now reach their backend over a session bearer (c98eddf), but each still declared SESSION_AUTH_FEATURE. Constructing one with a session token and no bearer therefore failed fast, and the disclosed scope claimed those services reject a session. Both are now wrong: the session-aware factory always supplies the bearer. Remove SESSION_AUTH_FEATURE from the five clients and record them in the BEARER_CAPABLE_CLIENTS allowlist that check-sentinel-guards enforces. SESSION_UNSUPPORTED_FEATURES shrinks to the three paths that really need a static token: kbagent kai, semantic-layer token --encrypt, and the importable SDK Client. auth login / auth register-projects disclose this shorter list, shipped as session_unsupported_features in --json. Drop the default_* client factories the services no longer call and the tests that constructed them. The SESSION_AUTH_FEATURE mechanism stays on BaseHttpClient as the way to mark a future client static-token-only, now tested against a synthetic subclass instead of a real one.
The auth docs and the kbagent skill/agent references still said a session covers "Storage + Manage only" and that everything else fails fast. CLI-13 made the Scheduler, Data Streams, Data Science, the semantic-layer group, the AI Service and Storage bucket sharing accept a session, so that framing misleads both a human reader and the kbagent agent. Rewrite the scope in docs/auth.md (the capability table plus the flow and config notes), docs/web-server.md, and the skill references (gotchas.md, auth-workflow.md, commands-reference.md), plus the keboola-expert agent and CLAUDE.md. Each now says a session works with almost every command and names the three features that still need a static token: kbagent kai, semantic-layer token --encrypt, and the importable SDK. The multi-project errors[] examples no longer imply a session fails those readers.
keboola-pr-reviewer-bot
left a comment
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 3/5) · profile keboola-mcp-server
Broad auth-surface change expanding browser-login session support across six service clients — well-built and tested, but the auth boundary warrants a human.
Concerns:
src/keboola_agent_cli/services/sharing_service.py: resolve_master_token drops static-token guard; session now forwarded to server for master checksrc/keboola_agent_cli/services/base.py: New generic session-aware factory extends bearer auth to five service clients at once
Suggested reviewers: @keboola/ai-swimlane-kai-assistant
CLI-13 widened session support to the Scheduler, Data Streams, Data Science, semantic-layer, AI Service and bucket-sharing clients, and shrank SESSION_UNSUPPORTED_FEATURES to three entries. Several docs and docstrings still described the old "Storage + Manage only" scope. - context.py AGENT_CONTEXT still listed the now-working services as fail-fast. `kbagent context` loads it as the agent's primary reference, so the stale copy makes an agent refuse commands that now work. Rewrite it to the three-entry list. - Shrink the same stale list in the SessionAuthUnsupportedError docstring (errors.py) and the _render_session_restrictions docstring (auth.py). - Tag the rewritten gotchas.md bullet (since vNEXT). An agent on an older kbagent must not read the widened support as always-present. - Drop six now-dead FEATURE_ALIASES entries in check_sentinel_guards.py. CLI-13 removed their SESSION_AUTH_FEATURE guards. - Add request-level tests. Each session-aware service client sends Authorization: Bearer plus X-KBC-ProjectId and drops X-StorageApi-Token on a real request. The static path is the mirror image.
zajca
left a comment
zajca
left a comment
There was a problem hiding this comment.
Actionable findings from the automated review.
| "DeveloperPortalClient", # own username/password identity, never a project token | ||
| # CLI-13: these reach their backend over a bearer via | ||
| # make_session_aware_client_factory; the backends accept OAuth tokens. | ||
| "SchedulerClient", |
There was a problem hiding this comment.
Reviewed by Opus.
The five clients are added to BEARER_CAPABLE_CLIENTS (so Check 2 skips them) and their SESSION_AUTH_FEATURE declarations are deleted (so the http_base.py:131 runtime guard is inert for them), but they are not added to PROJECT_CREDENTIAL_CLIENTS (line 116), which is the only seed Check 4 walks (_descendants_of(root, set(PROJECT_CREDENTIAL_CLIENTS)), line 392). These clients are constructed from project.token, which is exactly the value that can be a kbc-session:// sentinel — the situation Check 4's own docstring describes for KeboolaClient.
Verified on this head commit:
$ uv run python -c "from keboola_agent_cli.ai_client import AiServiceClient; \
c=AiServiceClient(stack_url='https://connection.keboola.com', token='kbc-session://10105'); \
print(repr(c._client.headers.get('x-storageapi-token')))"
'kbc-session://10105'
Before this PR the same call raised SessionAuthUnsupportedError. Today every call site goes through make_session_aware_client_factory, so nothing is broken now — but the PR removes the runtime net without extending the static one, so the next direct construction or hand-rolled default factory lands silently and sends the literal sentinel string as a credential (an opaque 401), which is precisely the drift check_sentinel_guards.py exists to stop.
Suggested fix: add SchedulerClient, StreamClient, DataScienceClient, MetastoreClient and AiServiceClient to PROJECT_CREDENTIAL_CLIENTS. Note that client/stream.py:34 (StreamClient(stack_url=..., token=self._token, http_auth=self._http_auth)) will then be flagged and needs an explicit, reasoned entry in the allowlist — it inherits the parent client's bearer hook and is safe.
| headers: dict[str, str] = {} | ||
| if http_auth is None: | ||
| headers["X-StorageApi-Token"] = token | ||
| super().__init__(base_url=base_url, token=token, headers=headers, http_auth=http_auth) |
There was a problem hiding this comment.
Reviewed by Opus.
MetastoreClient._do_request (lines 104-137) exists specifically because the metastore answers a valid non-master token with HTTP 401 "Failed to create project scope" — a permission gate, not a credential failure, remapped to MISSING_MASTER_TOKEN. Routing the client through http_auth puts BearerAuth.auth_flow (auth/token_provider.py:468-475) underneath it, and that hook retries any 401 after calling provider.force_refresh(token), which performs a real refresh-token exchange and rotates the refresh token in auth.json.
Failure scenario: a browser-login user whose session is not project-admin runs kbagent semantic-layer list. The metastore returns the project-scope 401; BearerAuth burns a network refresh + rotation and resends; the second 401 arrives and only then is the diagnosis produced. Every such call costs one pointless rotation and a doubled round trip. Worse, if that forced refresh itself fails (concurrent rotation by another kbagent process, revoked session), the user gets a "session expired — run kbagent auth login" error instead of the accurate MISSING_MASTER_TOKEN remedy, sending them to re-login for a problem re-login cannot fix.
The same 401-retry semantics already applied to Storage/Manage, but the metastore is the one client with a documented 401 that is not an auth failure, so it is worth either short-circuiting the retry for this exception string or noting the interaction where the http_auth wiring is added. Related: the remapped message still interpolates self._masked_token, which is "***" for a session client (mask_token("")), and advises kbagent project edit --token ..., which is not the right remedy for a session project.
soustruh
changed the title
O001: The drift guard (check_sentinel_guards.py) stopped covering the five clients CLI-13 made bearer-capable, so it no longer catches a session sentinel sent as a static token. Nothing leaked in practice, but the safety net was gone. Re-add the clients to the guard, with a test. O002: The metastore answers a valid non-master token with a 401. BearerAuth mistook that for an expired token and force-refreshed, wasting a token rotation and hiding the real "needs a master token" error. Skip the refresh for the metastore.
keboola-pr-reviewer-bot
left a comment
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 4/5) · profile keboola-mcp-server
Auth-surface change: extends session bearer tokens to 5 service clients and drops fail-fast static-token guards, so a human should confirm the accept/reject shift.
Impact flags: possible rollback re-introduction — see Check Run summary.
Suggested reviewers: @keboola/ai-swimlane-kai-assistant
zajca
left a comment
zajca
left a comment
There was a problem hiding this comment.
No actionable findings were found by the automated review.
…re helper SyncService imported `_default_ds_client_factory`, which keboola#759 (CLI-13) removed in favor of `make_session_aware_client_factory`. The import came from keboola#752 (CLI-8), valid on its own base. Both PRs passed CI alone; the combination broke `import sync_service`, so the whole CLI failed to load on main. Build the factory like DataAppService does. Static-token projects keep the same behavior; session projects now reach the DS API too.
Why
kbagent registers a browser-login project with a
kbc-session://sentinel token, but only the Storage and Manage clients accepted it. Every other client required a static Storage token and refused a session, even though those backends already accept an OAuth bearer token (rollout under DMD-1593, Done). The block was only on the kbagent side and was tracked nowhere.What changed
services/base.pyaddsmake_session_aware_client_factory(config_store, client_cls): it builds anyBaseHttpClientsubclass withBearerAuthfor akbc-session://project and with a static token otherwise.make_client_factorydelegates to it. Five service clients (SchedulerClient,StreamClient,DataScienceClient,MetastoreClient,AiServiceClient) accepthttp_authand drop theX-StorageApi-Tokenheader when it is set, the same asManageClient.A browser-login project now reaches these interfaces:
flow schedule,flow schedule-removestreamcommand groupdata-appcommand groupsemantic-layercommand group, exceptsemantic-layer token --encrypt(see Out of scope)docs query,config new,config examples,component detail,flow new/update/validatesharing share,sharing unshare:resolve_master_tokenno longer refuses a session, and the Storage API enforces the master privilegeconfig oauth-urlanddata-app git-credentialsalready worked for a master session: both check the token's realisMasterTokenprivilege, not whether it is static or a session. No change.The five clients no longer declare
SESSION_AUTH_FEATURE, and they are recorded in theBEARER_CAPABLE_CLIENTSallowlist thatcheck-sentinel-guardsenforces.SESSION_UNSUPPORTED_FEATURESshrinks to the three paths that still need a static token, andauth login/auth register-projectsdisclose that shorter list.Still to do
kbagent kai: it goes through the externalkai_clientlibrary, which takes a plain Storage token and has no bearer path, sokai_servicekeeps its static-token guard until the library accepts one.Out of scope
semantic-layer token --encryptstays static-token-only by design: it encrypts the project's own token so a metastore component can store and reuse it later, and akbc-session://sentinel is not a reusable credential.keboola_agent_cli.Client) stays static-token-only by design: it is a stateless entry point with no config directory and no session store to read a bearer from.org setup,feature *,project refresh/invite/member-*). A session has no Manage API access by design (DMD-1711); these need a manage token, the same as a static-token project.Testing
make checkpasses.TestSessionAwareServiceClientsbuilds each of the five clients through the session-aware factory and checks that a static token keeps theX-StorageApi-Tokenheader, while a session token drops it and gets a bearer. Live-verified on a real stack: the scheduler (read → 200; the activate write path reaches its permission check → 404, not 403), andstream/data-app/semantic-layer/docs queryread commands return ok with a session token where the released build refuses with "does not support browser-login".Refs: CLI-13; backend scheduler#145, mcp-server#406, sandboxes-service#625, keboola-as-code#2621.