Skip to content

Support ClickHouse cluster interserver-secret authentication#86

Open
BorisTyshkevich wants to merge 11 commits intofeature/unified-toolsfrom
feature/interserver-auth
Open

Support ClickHouse cluster interserver-secret authentication#86
BorisTyshkevich wants to merge 11 commits intofeature/unified-toolsfrom
feature/interserver-auth

Conversation

@BorisTyshkevich
Copy link
Copy Markdown
Collaborator

Summary

  • Lets altinity-mcp drop the shared ClickHouse password requirement by authenticating as a trusted cluster peer with a shared <secret>
  • Each query runs on ClickHouse as the MCP-authenticated user (OAuth subject in gating mode, or the configured clickhouse-username) so system.query_log attributes queries to the real identity
  • New CLI flags: `--clickhouse-cluster-name`, `--clickhouse-cluster-secret` (env: `CLICKHOUSE_CLUSTER_NAME`, `CLICKHOUSE_CLUSTER_SECRET`)

Depends on

Altinity/clickhouse-go#1 — the driver-side protocol extension. This PR's `go.mod` uses a local-path replace during development:

```
replace github.com/ClickHouse/clickhouse-go/v2 => /Users/Workspaces/altinity/clickhouse-go
```

Before merging, pick one resolution:

  1. Land Add cluster interserver-secret authentication clickhouse-go#1 → cut a tag on the fork with `module github.com/Altinity/clickhouse-go/v2` → pin `replace` to that tag
  2. Get the change accepted upstream and drop the `replace` entirely

Design

  • Interserver auth is TCP-only (ClickHouse has no HTTP equivalent); `validateClusterSecretConfig` rejects any other protocol and requires a non-empty cluster name
  • `pkg/clickhouse/client.go` wires the new `ClusterCredentials` into `clickhouse.Open` and clears the password so only the secret reaches the wire
  • `pkg/server/server.go` (GetClickHouseClientWithOAuth): when the cluster secret is set and OAuth claims are present, override `Username` with `claims.Subject` so the per-request identity propagates into ClickHouse

Test plan

  • `go build ./... && go vet ./...` — clean
  • `TESTCONTAINERS_RYUK_DISABLED=true go test ./pkg/config/ ./pkg/clickhouse/` — all pass
  • New `pkg/clickhouse/cluster_secret_test.go` spins up `clickhouse/clickhouse-server` with a `<remote_servers>` cluster and passwordless `alice`/`bob` users:
    • `alice`: `SELECT currentUser()` returns `alice` (impersonation works)
    • `bob`: switching `Username` switches effective user
    • `wrong_secret_rejected`: ClickHouse drops the query when the secret doesn't match
    • `NoPasswordRequired`: setting `Password` to garbage is ignored; only the secret authenticates
  • Manual smoke test with a real gated deployment once Add cluster interserver-secret authentication clickhouse-go#1 is tagged
  • Reviewer to confirm the OAuth-subject → ClickHouse-user mapping policy (currently uses `claims.Subject` verbatim — could be made configurable)

🤖 Generated with Claude Code

BorisTyshkevich and others added 11 commits April 17, 2026 12:12
Changes the MCP HTTP transport endpoint from /http to / (root). SSE stays
at /sse. OAuth paths stay under /oauth/*.

Go 1.22+ ServeMux gives literal routes precedence over wildcards, and /
acts as a catch-all for unmatched requests. /health, /openapi/*, /oauth/*,
and /.well-known/* all keep their specific registrations; unknown paths
fall through to the MCP handler which rejects non-protocol traffic cleanly.

Also removes 6 redundant /http-prefixed OAuth discovery aliases that only
existed because clients hit /.well-known/*/http when the transport was
under /http. With the transport at root, the standard /.well-known/* paths
already work — the aliases become noise.

transportRoutePatterns accepts an empty transport string to produce root
patterns (/ and /{token}); the HTTP transport call sites now pass "".
SSE still passes "sse" and is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Follows the HTTP transport moving from /http to root path.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Stops requiring a shared ClickHouse username/password for altinity-mcp's
gated deployments. When `clickhouse-cluster-secret` is set, altinity-mcp
handshakes with ClickHouse as a trusted cluster peer using the shared
secret and executes each query as the MCP-authenticated user (taken from
OAuth claims or the configured `clickhouse-username`).

- Add `ClusterName`/`ClusterSecret` to `ClickHouseConfig` with matching
  CLI flags (`--clickhouse-cluster-name`, `--clickhouse-cluster-secret`)
  and `CLICKHOUSE_CLUSTER_*` env vars.
- Reject invalid combinations early: interserver auth is TCP-only and
  requires a non-empty cluster name.
- Wire `ClusterCredentials` through `clickhouse.Open` and drop the
  static password when the secret is set, so only the secret reaches
  the wire.
- In OAuth gating mode, override `chConfig.Username` with the OAuth
  `Subject` when the cluster secret is active so `system.query_log`
  attributes the query to the end user.

Requires the Altinity/clickhouse-go fork which implements the protocol
extension (see Altinity/clickhouse-go#1). The `go.mod` uses a local-path
replace during development — see the in-file comment for the follow-up
resolution path.

Adds `pkg/clickhouse/cluster_secret_test.go` which spins up
`clickhouse/clickhouse-server` with a `<remote_servers>` cluster and
passwordless users (`alice`, `bob`), then verifies `SELECT currentUser()`
returns the impersonated user, that switching Username switches the
effective user, and that a wrong secret is rejected.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds a "Cluster-secret authentication (optional)" subsection to the
gating mode docs covering:

- Why: removes the shared static password, attributes queries to the
  real end-user in `system.query_log`
- Full altinity-mcp + ClickHouse `<remote_servers>` config snippets
- The user/role precreation model (no auto-provisioning — impersonated
  users and their grants must exist on ClickHouse ahead of time)
- Limitations: TCP-only, no `external_roles` forwarding, secret-hygiene
  requirements

Also updates the Requirements bullet to call out the TCP-only constraint
for cluster-secret gating.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Testers on the blocker path reported that `claims.Subject` is an opaque
numeric ID for Google (21 digits) and a UUID for Keycloak, which defeats
per-user attribution in `system.query_log` and forces operators to
provision ClickHouse users with cryptic keys.

Use `claims.Email` first, fall back to `claims.Subject` only when the
IdP omits email (machine-to-machine flows, some restricted scopes).
Matches the convention used by forward mode's `username_claim: email`
setups so a single pool of pre-provisioned CH users works across both
modes.

Docs updated to reflect the email-first lookup order.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Needed to run e2e tests that exercise the HTTP transport at the root path.
Will be dropped automatically once #83 merges to main and this branch
rebases onto main.
The /oauth/register endpoint returned only `authorization_code` in
`grant_types`, even though:

- `/.well-known/oauth-authorization-server` already advertised
  `refresh_token` via `grant_types_supported`
- the `/oauth/token` endpoint already accepted `grant_type=refresh_token`
- the auth_code exchange already minted a `refresh_token` in the
  response body

Per RFC 7591 §3.2.1, the `grant_types` list in the registration
response is authoritative for the client. Strict clients (observed with
Claude.ai) treat an omitted grant as forbidden and silently skip the
refresh flow — forcing end users to re-authorize through the browser on
every new chat session.

Include `refresh_token` alongside `authorization_code` so the
registration response matches server capabilities. Add a regression
assertion to the existing `dynamic_client_registration` subtest that
pins both grants in the response body.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Brings the refresh_token grant advertisement fix into the
interserver-auth test build so Claude.ai can exercise the full refresh
flow against gated cluster-secret deployments.

Will be dropped automatically once #88 merges to main and this branch
rebases onto main.
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