[jbiros-keboola]

What

Storage API tokens are project-bound and travel in X-StorageApi-Token. Programmatic tokens — kbc_at_* session tokens and kbc_pat_* personal access tokens — are not bound to a project, so they must be sent as Authorization: Bearer together with the project id in X-KBC-ProjectId.

from kbcstorage.client import Client, BearerToken

client = Client('https://connection.keboola.com', 'your-token')                    # unchanged
client = Client('https://connection.keboola.com', BearerToken('kbc_at_...', 1234)) # bearer

How

kbcstorage/auth.py holds two strategies, StorageApiToken and BearerToken, plus coerce(), which normalises the token argument into a strategy object. A bare string still becomes a StorageApiToken, so every existing caller is unaffected — including positional construction and directly built endpoints.

Endpoint keeps the strategy on self.auth and derives its header block from auth.headers(). self.token still exposes the raw string.

Endpoints that other endpoints build internally now receive self.auth rather than the raw token, so the scheme propagates through the multi-step paths in tables.py, workspaces.py and buckets.py.

Two fixes fell out of this:

• Buckets.list_tables hardcoded X-StorageApi-Token, which under a bearer token would have sent both schemes on one request.
• The four request verbs now merge caller headers into a new dict rather than mutating the caller's, which headers.update() did.

Tests

73 mock tests pass; 14 are auth-specific. Both schemes are asserted mutually exclusive — each test checks the other scheme's header is absent, so a regression sending both would fail rather than pass quietly.

Covered: every branch of auth.py; Endpoint header construction; all 10 client endpoints plus nested .metadata endpoints under both schemes; Buckets.list_tables; and bearer propagation into the Tokens endpoint that Workspaces.create builds internally.

Two known gaps, neither a defect — the call sites are correct, but a future refactor could break them silently:

• test_base.py exercises _get only; _post/_put/_delete took the same header change untested.
• tables.py has seven sites building Files/Jobs from self.auth with no bearer test, as does buckets.py:122. test_workspaces.py sets the precedent for how to cover these.

Unrelated commits on this branch

Two housekeeping commits precede the feature and can be reviewed independently, or split out if you'd prefer:

• 0b53bc2 — ignore .venv, drop the obsolete compose version key
• 6dfcab4 — document local development setup; declare flake8 in a dev extra; exclude .venv from flake8 and the Docker build context; fix the Tests command, which named the image rather than the ci service and failed with no such service

🤖 Generated with Claude Code

[linear-code]

UT-5603

[Copilot]

🟡 Changes recommended

Resolve the Python-version compatibility issues and prevent warnings during valid bearer-token usage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds strategy-based authentication for Storage API and programmatic bearer tokens, with propagation through nested endpoints and updated tests and development tooling.

Changes:

• Adds StorageApiToken and BearerToken authentication strategies.
• Propagates authentication through nested endpoints and improves header handling.
• Updates tests, documentation, and local development configuration.

File summaries

File	Summary
tests/mocks/test_workspaces.py	Tests bearer authentication propagation
tests/mocks/test_client.py	Tests endpoint authentication
tests/mocks/test_buckets.py	Tests exclusive authentication headers
tests/mocks/test_base.py	Tests base header behavior
tests/mocks/test_auth.py	Tests authentication strategies
README.md	Documents authentication and development setup
pyproject.toml	Adds development dependency
kbcstorage/workspaces.py	Propagates authentication strategy
kbcstorage/tables.py	Propagates authentication strategy
kbcstorage/configurations.py	Propagates authentication strategy
kbcstorage/client.py	Adds strategy-aware client setup
kbcstorage/buckets.py	Uses strategy-aware requests
kbcstorage/base.py	Builds authentication headers
kbcstorage/auth.py	Implements authentication strategies
Dockerfile	Installs development extras
docker-compose.yml	Removes obsolete version key
.gitignore	Ignores virtual environments
.flake8	Excludes virtual environments
.dockerignore	Excludes virtual environments

Review details

• Files reviewed: 18/19 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jbiros-keboola]

Fixed in d4d0b69. requires-python is now >=3.10 and the README matches, naming int | str in workspaces.load_tables as the reason. Added the 3.10 and 3.11 classifiers too. Both the annotation and the >=3.7 predate this PR — this branch only newly asserted 3.7 in the README, so the fix goes here.

[jbiros-keboola]

Fixed in d4d0b69. coerce() warned on every BearerToken it received, including ones already constructed, so it fired 13 times for a single Client(url, BearerToken(...)) and again per Tables.load / Workspaces.create call. Both warnings are removed along with _caller_stacklevel(); the caveat stays in the .token docstrings on Endpoint and Client, which is where it belongs. tests/mocks/test_auth.py::TestCoerce::test_no_warnings asserts coerce() is silent for all three input forms, and the mock suite now passes under -W error.

[jbiros-keboola]

Resolved in d4d0b69 from the other direction: rather than loosening the flake8 range, requires-python is now >=3.10, which was already the real floor — workspaces.load_tables uses int | str annotations that fail at import below 3.10. flake8>=7 needs 3.8.1+, so the conflict is gone. Docker and CI both run 3.11.

[jbiros-keboola]

@jirkasemmler can you review it please when you find time? There is public API breakign change.

[jbiros-keboola]

@jirkasemmler can you review it pleas, if not at least let me know I will find someone else.

[jirkasemmler]

on today's list

[jirkasemmler]

Code review

The design is good. Passing the auth strategy through the internally built endpoints is the right approach, and the Buckets.list_tables and header-mutation fixes are correct. A few things to address before merging:

1. BearerToken emits a UserWarning on the documented happy path. coerce() warns every time it receives a BearerToken, and every endpoint calls it. Client(url, BearerToken(...)) by itself produces 13 warnings, and more follow at runtime because Tables.load, Workspaces.create and similar methods build new Files/Jobs/Tokens endpoints on each call. With python -W error or pytest filterwarnings = error, constructing the client raises. The note about .token belongs in the .token docstring (it is already there), not in a runtime warning.

sapi-python-client/kbcstorage/auth.py

Lines 81 to 91 in 36673e2

	if isinstance(token, BearerToken):
	warnings.warn(
	"Authenticating with a bearer token: `.token` returns the "
	"programmatic token, not a Storage API token, and will be "
	"rejected if sent as X-StorageApi-Token. Pass `.auth` when "
	"building further endpoints.",
	UserWarning,
	stacklevel=_caller_stacklevel(),
	)
	return token

2. Passing the token as a string now emits a DeprecationWarning. The PR description says "every existing caller is unaffected", and the README marks the string form as # unchanged. Because of _caller_stacklevel(), the warning is attributed to user code, so it will be shown there. Either document the deprecation (README and changelog) or drop the warning for now.

sapi-python-client/kbcstorage/auth.py

Lines 91 to 99 in 36673e2

	return token
	auth = StorageApiToken(token)
	warnings.warn(
	"Passing the token as a string is deprecated, "
	"pass kbcstorage.auth.StorageApiToken instead.",
	DeprecationWarning,
	stacklevel=_caller_stacklevel(),
	)
	return auth

3. Breaking change: kbc_at_/kbc_pat_ strings now raise ValueError. Client(url, 'kbc_pat_...') used to send the token as X-StorageApi-Token and now fails at construction. That may be intended, but it should be called out as a breaking change in the release notes, with a version bump to match.

sapi-python-client/kbcstorage/auth.py

Lines 34 to 40 in 36673e2

	# never echo the token itself, the message can end up in logs
	if token.startswith(PROGRAMMATIC_PREFIXES):
	raise ValueError(
	"A token with a 'kbc_at_'/'kbc_pat_' prefix is a programmatic "
	"token and cannot be sent as X-StorageApi-Token. Use "
	"kbcstorage.auth.BearerToken with the project id instead.")
	self.token = token

4. The Python 3.7 claim is inconsistent (also raised by Copilot). The README and requires-python say 3.7+, but flake8>=7 requires 3.8+, and Workspaces.load_tables uses int | str annotations, which fail at import time below 3.10. Please align requires-python/README with what is actually supported.

sapi-python-client/pyproject.toml

Lines 11 to 13 in 36673e2

	readme = "README.md"
	requires-python = ">=3.7"
	keywords = ["keboola", "storage"]

sapi-python-client/pyproject.toml

Lines 36 to 38 in 36673e2

	dev = [
	"flake8>=7,<8",
	]

🤖 Generated with Claude Code

[jbiros-keboola]

@jirkasemmler all four addressed in d4d0b69.

1. BearerToken warning on the happy path. Removed. The bug was the asymmetry — coerce() returned early for StorageApiToken but warned on every BearerToken it saw, including ones already constructed, so it fired once per endpoint rather than once per coercion. Measured 13 for a bare Client(url, BearerToken(...)), more per Tables.load / Workspaces.create call. The caveat stays in the .token docstrings. TestCoerce::test_no_warnings asserts silence for all three input forms, and the mock suite now passes under -W error — it raised at construction before.

2. DeprecationWarning on string tokens. Dropped rather than documented. The string form isn't actually going away: it's the ergonomic path for a Storage API token and there's no plan to remove it, so deprecating it would force a rewrite on every existing user for no benefit. README reworded from "kept for backward compatibility" to "fully supported and not deprecated", and the PR description's "every existing caller is unaffected" is true again. _caller_stacklevel() went with it — auth.py is down 48 lines with no warnings import left.

3. Breaking change. Intended, now a README section under Authentication. Worth noting what it replaces: the prior behaviour sent kbc_pat_* as X-StorageApi-Token and got a 401 back, so this moves the failure from request time to construction time with a message that names the fix. Version comes from git tags via setuptools-git-versioning and the README says 0.11.0 — I've left the tag itself to you as a release step.

4. Python version. requires-python = ">=3.10", README matches and names int | str in workspaces.load_tables as the reason, plus 3.10/3.11 classifiers. That resolves Copilot's flake8 point too, since flake8>=7 needs 3.8.1+. Both the annotation and the >=3.7 predate this PR — the branch only newly asserted 3.7 in the README, so I fixed it here rather than punting.

77 mock tests pass, and pass under -W error.

🤖 Generated with Claude Code

[jirkasemmler]

Re-review after d4d0b69

All four points from my previous review are resolved and verified: auth.py has no warnings import and the mock suite passes under python -W error (77 tests), the string form is not deprecated, the kbc_at_/kbc_pat_ rejection is documented under "Breaking change in 0.11.0", and requires-python/README agree on 3.10. Flake8 is clean. I also checked the backend contract in keboola/connection: BearerTokenAuthenticator reads X-KBC-ProjectId, and kbc_at_/kbc_pat_ are exactly the prefixes the Storage routes accept (kbc_apc_, kbc_rt_, kbc_mfa_ are not storage bearers), so the client-side guard matches.

Approving. The rest is minor and can be addressed here or deferred.

Minor

1. README shows two canonical forms for a Storage API token. The lead example at README.md:21-23 was changed to StorageApiToken('your-token'), while the Authentication section says the bare string is fully supported. I would keep the bare string in the lead example (and in "Endpoint Classes Usage") and introduce StorageApiToken only in the Authentication section.
2. The Python floor bump is an install-time break for 3.8/3.9 and is not listed under "Breaking change". It is justified (int | str at workspaces.py:233 already failed at import below 3.10), but one line in the breaking-change section would help users whose pipelines pin 3.9.
3. _post/_put/_delete header merging is untested (base.py:132,157,182). test_caller_headers_are_kept_and_not_mutated could be parametrized over the four verbs.
4. Endpoint.token is now a read-only property (base.py:58-67). Assigning endpoint.token = ... worked on 0.10.0 and now raises AttributeError. No practical impact (it never affected _auth_header), but worth a line in the release notes.

Nits

• base.py:47 duplicates the Token is required. guard that coerce() already raises.
• coerce() with a non-string token (bytes, int, Mock()) fails with AttributeError from startswith at auth.py:14; an isinstance(token, str) check would give a clear TypeError.
• BearerToken(tok, True) and BearerToken(tok, 1234.0) send True/1234.0 as X-KBC-ProjectId; the backend rejects both with "must be a positive integer" and the client gives no hint.
• Client._token (client.py:35) is a second source of truth beside _auth; token could just return self._auth.token like Endpoint.token does.
• PR description still says 73 tests; it is 77 now.

Notes

• Bearer auth is feature-gated server-side: BearerTokenAuthenticator::supports() returns false on stacks without programmatic-auth, so BearerToken gets a 401 there. Worth one sentence in the README Authentication section.
• The functional suites run only with a Storage token, so the bearer path is mock-covered only. The backend E2E confirms tokens/verify under bearer resolves to the admin's master storage token, so Workspaces._get_default_backend should work, but this repo does not prove it.
• Version comes from git tags and README already says 0.11.0; tag 0.11.0 after merge.

🤖 Generated with Claude Code