Skip to content

feat(authn): require credentials and bind queries to their subject - #14

Merged
ekalinin merged 1 commit into
fix/transport-contractfrom
feat/transport-auth
Aug 20, 2026
Merged

feat(authn): require credentials and bind queries to their subject#14
ekalinin merged 1 commit into
fix/transport-contractfrom
feat/transport-auth

Conversation

@ekalinin

@ekalinin ekalinin commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Stacked on #13.

Problem

There was no authentication middleware on any transport and no securityScheme in the OpenAPI document. POST /v1/admin/reload and GET /v1/queries/{id}/result were open along with everything else. In practice that is unauthenticated execution of arbitrary SQL under the service credentials of every configured database, plus the ability to stop other people's queries, read their results and reload the process.

Separately: knowing a query_id was enough to read someone else's SQL, status, stats and result.

Mechanism

Static bearer tokens from auth.tokens. The value is normally read from an environment variable (value_env), so it never has to live in the config file - in Kubernetes, in a ConfigMap. Comparison is constant time across all tokens with no early exit, so neither timing nor control flow reveals which token was close.

Scopes: read (status, stats, download, list, watch), write (submit, stop), admin (reload, can-stop, /metrics). write implies read - a token that may submit a query has to be able to poll it and fetch its result, or only mode: sync works - and admin implies both.

internal/authn holds only the identity, the scopes and the token comparison, so the core can stamp the submitting subject onto a record without linking a transport. The chi middleware lives in internal/transport/rest and the Connect interceptor in internal/transport/grpcconnect; go list -deps ./internal/core/manager | grep connectrpc is empty.

Wiring

The /v1 subtree denies by default: the scope middleware is mounted with r.Use on the subtree rather than r.With per route, so a route added later cannot come out unauthenticated by omission. That matters more than the 401 it saves - a route without a gate would also read the records of every subject, because a missing identity is what "authentication is disabled" looks like from the inside. Groups raise the bar to write where a route needs it.

/metrics needs admin wherever it is mounted: its labels enumerate every configured db_id and the traffic volume per database, and admin_addr is optional, so the default deployment has it on the public listener. The shipped Prometheus config carries a token.

Connect gets an interceptor covering both unary and streaming calls - DownloadResult and WatchQuery are server streams that go through a different hook and would otherwise stay open. An unmapped procedure defaults to admin, so a newly added RPC is never public by oversight.

A browser cannot set the Authorization header on a WebSocket handshake, which would have made /v1/ws unreachable for exactly the clients the Origin check exists for. The credential may also be offered as the subprotocol pair ["dbbridge.bearer", token]; the server selects and echoes only the marker, never the token.

401 and 403 go through the same JSON envelope as every other error, with the request ID, instead of http.Error text, and both are declared in OpenAPI. The admin router shares the public router's middleware, so admin errors carry a request ID, admin requests are logged and a panic there comes back as a 500.

/healthz and /readyz stay open. An auth section that resolves to no usable token is a startup failure: coming up with the API open while the operator believes it is protected is worse than not coming up at all.

auth is reported under report.ignored by a reload rather than silently kept, because the Authenticator is built once - so revoking a leaked token takes a restart, and the reload no longer claims success while the token keeps working.

Binding queries to their subject

QueryRecord.Subject is filled from the authentication context and checked in status, stats, download, stop and watch. admin sees everything. A foreign query answers 404 rather than 403, so the API does not confirm that an ID exists. Records written before subject binding have no subject and are reachable only with admin - so turn authentication on while the instance is idle, or expect queries submitted before the switch to answer 404 to their owners until result_ttl expires.

The idempotency key is namespaced by the subject that chose it. StartQuery is the one read path that does not go through the authorization check, and the key was global per database, so a caller who sent somebody else's key got their whole record back - SQL text, stats, owner and result locator - while its own SQL was never run, and could hold the key for its full TTL. I3 only has to hold inside a subject.

When credentials are configured, a request that reaches the service without an identity is denied rather than treated as "authentication is off": that is the failure mode a forgotten gate would otherwise open.

Admin isolation

server.admin_addr moves /metrics and /v1/admin/* to their own listener: the metric labels enumerate every configured db_id and the admin routes reload the process, so neither belongs on the public port. That is network isolation, not authorization - the admin scope is still required there.

Tests

Unit tests for authn: rejection of every unusable configuration (no tokens, no value, no subject, no scopes, unknown scope, empty environment variable, duplicate value), reading a value from the environment, header parsing, the scope implications, and the AuthorizeSubject matrix. Procedure mapping moved to grpcconnect with the interceptor.

Transport tests: 401 without a token and with a bad one, 403 with an insufficient scope, 200 with the right one, open probes, a stream without a token; the owner reads its own query while another subject gets 404 and admin gets 200, over REST and over Connect; the same key from two subjects returns two different queries and each caller's own SQL; /metrics answers 401/403/200 by scope; the separate admin listener still requires admin; a WebSocket handshake with and without a subprotocol credential; the 401 envelope carries request_id and Vary: Authorization; and config rejects an empty auth.tokens.

Verification

go test -race ./..., golangci-lint run ./... - clean. OpenAPI gains securitySchemes.bearerAuth, reusable 401/403 responses on every authenticated operation, and a global security with the probes exempted.

There was no authentication on any transport and no securityScheme in the
OpenAPI document. Anyone who could reach the port could run arbitrary SQL
under the service credentials of every configured database, stop other
people's queries, read their results and reload the process.

internal/authn holds static bearer tokens loaded from auth.tokens, with the
value normally taken from an environment variable so it never has to sit in a
ConfigMap. Comparison is constant time across all tokens with no early exit.
Scopes are read (status, stats, download, list, watch), write (submit, stop)
and admin (reload, can-stop); admin implies the others. The package sits
outside internal/transport because the core stamps a query with the subject
that submitted it and must not import a transport.

REST gates every /v1 route on its scope, the Connect handler gets an
interceptor that covers unary and streaming calls alike, and an unmapped RPC
defaults to admin so a new method is never accidentally public. The health
probes stay open; a configured auth section that resolves to no usable token
is a startup failure rather than a silent pass-through.

Knowing a query ID was also enough to read anyone's SQL, status, stats and
result. QueryRecord now carries the submitting subject, and status, stats,
download, stop and watch check it. Admin acts across subjects; a record
written before subject binding has no owner and is admin-only. A foreign
query answers 404, not 403, so the API does not confirm that an ID exists.

Finally, /metrics and /v1/admin/* move to their own listener when
server.admin_addr is set: the metric labels enumerate every configured db_id
and the admin routes reload the process, so neither belongs on the public
port.
@ekalinin
ekalinin force-pushed the feat/transport-auth branch from 3bedb44 to cb77494 Compare August 18, 2026 09:47
@ekalinin
ekalinin merged commit 258e9bd into fix/transport-contract Aug 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant