From 41647a7f47b9698d2c8d01bbb1a1c771266adccc Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 15 Jun 2026 15:18:43 +0200 Subject: [PATCH 1/5] feat(oauth): activate filtered ClickHouse roles per request from a JWT claim Implements #140. In OAuth mode, when oauth.role_claim is set, read that JSON-array claim from the validated token, keep only role names matching oauth.role_filter, and activate exactly that set per ClickHouse request via repeated HTTP `role=` query params. Only ever narrows the user's active roles; never grants. - config.ClickHouseConfig.Roles: per-request roles (never from CLI/file). - GetClickHouseClientWithOAuthForConfig: read+filter roles, fail closed with access-denied on an empty set (no fallback to the full grant), refuse on non-HTTP protocol. Applies to single- and multi-cluster paths. - newClientWithOAuth: the Bearer-vs-Basic auto-detect probe stays role-free so an ungranted role's 497 can't be misread as an auth-method mismatch; the detected method then builds the real role-carrying client. - clickhouse.connect(): install a TransportFunc RoundTripper that appends one `role=` param per role (Settings can't express repeated params). - Startup validation requires + compiles role_filter when role_claim is set. Tests: roleRoundTripper unit test, embedded-CH currentRoles()/ACCESS_DENIED integration, server-layer fail-closed/TCP-guard/gate-pass. NOTE: go.mod carries a temporary `replace => ../go-mcp-oauth-sdk` for the unmerged SDK branch; drop it and bump the require before the PR. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/altinity-mcp/main.go | 13 +++ go.mod | 2 + pkg/clickhouse/client.go | 40 ++++++- pkg/clickhouse/client_roles_test.go | 142 +++++++++++++++++++++++++ pkg/config/config.go | 5 + pkg/server/server.go | 18 ++++ pkg/server/server_auth_oauth.go | 22 ++++ pkg/server/server_client.go | 47 ++++++-- pkg/server/server_client_roles_test.go | 74 +++++++++++++ 9 files changed, 354 insertions(+), 9 deletions(-) create mode 100644 pkg/clickhouse/client_roles_test.go create mode 100644 pkg/server/server_client_roles_test.go diff --git a/cmd/altinity-mcp/main.go b/cmd/altinity-mcp/main.go index 92e9a20..f895bcc 100644 --- a/cmd/altinity-mcp/main.go +++ b/cmd/altinity-mcp/main.go @@ -12,6 +12,7 @@ import ( "os" "os/signal" "reflect" + "regexp" "strings" "sync" "syscall" @@ -1175,6 +1176,18 @@ func validateOAuthRuntimeConfig(cfg config.Config) error { } } + // Per-request role activation: role_filter is the safety net that stops an + // over-broad/misconfigured role_claim from activating real-data roles, so + // it is required (and must compile) whenever role_claim is set. + if strings.TrimSpace(cfg.Server.OAuth.RoleClaim) != "" { + if strings.TrimSpace(cfg.Server.OAuth.RoleFilter) == "" { + return fmt.Errorf("oauth: role_filter is required when role_claim is set (it bounds which roles may be activated)") + } + if _, err := regexp.Compile(cfg.Server.OAuth.RoleFilter); err != nil { + return fmt.Errorf("oauth: role_filter is not a valid regex: %w", err) + } + } + return nil } diff --git a/go.mod b/go.mod index 854ce36..95b7583 100644 --- a/go.mod +++ b/go.mod @@ -45,3 +45,5 @@ require ( golang.org/x/sys v0.45.0 // indirect golang.org/x/text v0.37.0 // indirect ) + +replace github.com/altinity/go-mcp-oauth-sdk => ../go-mcp-oauth-sdk diff --git a/pkg/clickhouse/client.go b/pkg/clickhouse/client.go index 2464aaf..150f07b 100644 --- a/pkg/clickhouse/client.go +++ b/pkg/clickhouse/client.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "net/http" "os" "regexp" "strings" @@ -130,7 +131,7 @@ func (c *Client) connect() error { Password: c.config.Password, } - conn, openErr := clickhouse.Open(&clickhouse.Options{ + opts := &clickhouse.Options{ Addr: []string{fmt.Sprintf("%s:%d", c.config.Host, c.config.Port)}, Auth: auth, TLS: tlsConfig, @@ -143,7 +144,22 @@ func (c *Client) connect() error { MaxIdleConns: 5, ConnMaxLifetime: time.Hour, DialStrategy: dialWithoutQueryDeadline, - }) + } + + // Per-request role activation: append repeated `role=` query params to every + // HTTP request. The driver's Settings map is single-valued and serialized + // with url.Values.Set, so it can't express `?role=a&role=b`; wrapping the + // transport is the supported way. TransportFunc is HTTP-only in the driver + // (native TCP ignores it) — OAuth mode runs over HTTP, and the caller + // refuses role activation on non-HTTP protocols. + if len(c.config.Roles) > 0 { + roles := append([]string(nil), c.config.Roles...) + opts.TransportFunc = func(t *http.Transport) (http.RoundTripper, error) { + return &roleRoundTripper{wrapped: t, roles: roles}, nil + } + } + + conn, openErr := clickhouse.Open(opts) if openErr != nil { log.Error(). @@ -183,6 +199,26 @@ func dialWithoutQueryDeadline(ctx context.Context, connID int, opt *clickhouse.O return clickhouse.DefaultDialStrategy(context.WithoutCancel(ctx), connID, opt, dial) } +// roleRoundTripper appends a `role=` query param per configured role to every +// outgoing HTTP request, activating exactly those roles for the request +// (ClickHouse honours repeated role params, e.g. ?role=a&role=b). It wraps the +// driver's default transport; the request is cloned so the shared *http.Request +// is never mutated (the client may retry). +type roleRoundTripper struct { + wrapped http.RoundTripper + roles []string +} + +func (rt *roleRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + r2 := req.Clone(req.Context()) + q := r2.URL.Query() + for _, role := range rt.roles { + q.Add("role", role) + } + r2.URL.RawQuery = q.Encode() + return rt.wrapped.RoundTrip(r2) +} + func prepareHTTPAuthForClickHouse(cfg config.ClickHouseConfig) (map[string]string, clickhouse.GetJWTFunc) { if len(cfg.HttpHeaders) == 0 { return nil, nil diff --git a/pkg/clickhouse/client_roles_test.go b/pkg/clickhouse/client_roles_test.go new file mode 100644 index 0000000..2acff2a --- /dev/null +++ b/pkg/clickhouse/client_roles_test.go @@ -0,0 +1,142 @@ +package clickhouse + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/altinity/altinity-mcp/internal/testutil/embeddedch" + "github.com/stretchr/testify/require" +) + +// captureRT records the RawQuery of the request it receives and returns an +// empty 200 response. It stands in for the driver's real transport. +type captureRT struct{ gotQuery string } + +func (c *captureRT) RoundTrip(req *http.Request) (*http.Response, error) { + c.gotQuery = req.URL.RawQuery + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader("")), + Header: make(http.Header), + Request: req, + }, nil +} + +// TestRoleRoundTripper pins the multi-role wire encoding: each configured role +// is appended as its own repeated `role=` query param, existing params are +// preserved, and the caller's request is not mutated (so retries are safe). +func TestRoleRoundTripper(t *testing.T) { + t.Parallel() + cap := &captureRT{} + rt := &roleRoundTripper{wrapped: cap, roles: []string{"sandbox_mcp", "readonly_mcp"}} + + req, err := http.NewRequest(http.MethodPost, "http://ch:8123/?database=default", strings.NewReader("SELECT 1")) + require.NoError(t, err) + origQuery := req.URL.RawQuery + + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + // The shared request object must be untouched. + require.Equal(t, origQuery, req.URL.RawQuery) + + vals, err := url.ParseQuery(cap.gotQuery) + require.NoError(t, err) + require.Equal(t, []string{"sandbox_mcp", "readonly_mcp"}, vals["role"], "both roles sent as repeated params") + require.Equal(t, "default", vals.Get("database"), "pre-existing query params preserved") +} + +// roleTestUsersXML enables SQL access management on the default user so the +// test can CREATE ROLE / CREATE USER / GRANT. embedded-clickhouse ships no +// users.xml of its own, so we provide a minimal one. +const roleTestUsersXML = ` + + + + + ::/0 + default + default + 1 + + + + + +` + +// TestClientRoleActivation proves end-to-end that config.Roles activates only +// the named roles for a request: currentRoles() returns exactly the activated +// set, and a query needing a non-activated role's privilege is denied. +func TestClientRoleActivation(t *testing.T) { + t.Parallel() + // SQL CREATE ROLE/USER needs a writeable access storage; point + // access_control_path at a temp dir (users.xml alone is read-only). + accessDropIn := fmt.Sprintf( + "%s/", + t.TempDir(), + ) + cfg := embeddedch.Setup(t, + embeddedch.WithUsersXML(roleTestUsersXML), + embeddedch.WithConfigDropIn(accessDropIn), + ) // HTTP protocol by default + ctx := context.Background() + + admin, err := NewClient(ctx, *cfg) + require.NoError(t, err) + defer func() { require.NoError(t, admin.Close()) }() + + exec := func(q string) { + t.Helper() + res, err := admin.ExecuteQuery(ctx, q) + require.NoError(t, err, q) + require.Empty(t, res.Error, q) + } + exec("CREATE DATABASE IF NOT EXISTS roletest") + exec("CREATE TABLE IF NOT EXISTS roletest.public (x UInt64) ENGINE = Memory") + exec("CREATE TABLE IF NOT EXISTS roletest.secret (x UInt64) ENGINE = Memory") + exec("CREATE ROLE IF NOT EXISTS r_public_mcp") + exec("CREATE ROLE IF NOT EXISTS r_secret_real") + exec("GRANT SELECT ON roletest.public TO r_public_mcp") + exec("GRANT SELECT ON roletest.secret TO r_secret_real") + exec("CREATE USER IF NOT EXISTS ruser IDENTIFIED WITH no_password HOST ANY") + exec("GRANT r_public_mcp, r_secret_real TO ruser") + + // Base config for the restricted user, activating only the sandbox role. + userCfg := *cfg + userCfg.Username = "ruser" + userCfg.Password = "" + userCfg.Roles = []string{"r_public_mcp"} + + cl, err := NewClient(ctx, userCfg) + require.NoError(t, err) + defer func() { require.NoError(t, cl.Close()) }() + + t.Run("narrows_active_roles", func(t *testing.T) { + res, err := cl.ExecuteQuery(ctx, "SELECT arrayStringConcat(arraySort(currentRoles()), ',') AS roles") + require.NoError(t, err) + require.Empty(t, res.Error) + require.Len(t, res.Rows, 1) + require.Equal(t, "r_public_mcp", fmt.Sprint(res.Rows[0][0]), + "only the activated role is current, not the full granted set") + }) + + t.Run("activated_role_privilege_allowed", func(t *testing.T) { + res, err := cl.ExecuteQuery(ctx, "SELECT count() FROM roletest.public") + require.NoError(t, err) + require.Empty(t, res.Error) + }) + + t.Run("non_activated_role_privilege_denied", func(t *testing.T) { + _, err := cl.ExecuteQuery(ctx, "SELECT count() FROM roletest.secret") + require.Error(t, err, "secret table needs r_secret_real, which was not activated") + require.Contains(t, strings.ToLower(err.Error()), "privileg", + "expected an ACCESS_DENIED / not-enough-privileges error") + }) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index c573d80..b6498b1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -53,6 +53,11 @@ type ClickHouseConfig struct { MaxResultBytes int `json:"max_result_bytes,omitempty" yaml:"max_result_bytes,omitempty" flag:"clickhouse-max-result-bytes" env:"CLICKHOUSE_MAX_RESULT_BYTES" desc:"Per-request approximate byte cap on result body (0=default 50000, <0=disable)"` HttpHeaders map[string]string `json:"http_headers" yaml:"http_headers" flag:"clickhouse-http-headers" env:"CLICKHOUSE_HTTP_HEADERS" desc:"HTTP Headers for ClickHouse"` ExtraSettings map[string]string `json:"extra_settings,omitempty" yaml:"extra_settings,omitempty" desc:"Per-request ClickHouse settings injected by tool_input_settings"` + // Roles is the per-request set of ClickHouse roles to activate for this + // request (sent as HTTP `role=` query params). Populated at request time + // from the OAuth role_claim/role_filter; never set from CLI/file/env. Only + // narrows the user's active roles. HTTP protocol only. + Roles []string `json:"-" yaml:"-"` // MaxQueryLength caps the size in bytes of a single SQL query string sent by a client. // Default 10 MB when 0. Set to a negative number to disable the check. MaxQueryLength int `json:"max_query_length,omitempty" yaml:"max_query_length,omitempty" flag:"clickhouse-max-query-length" env:"CLICKHOUSE_MAX_QUERY_LENGTH" desc:"Max bytes of SQL query string accepted from clients (0=default 10MB, <0=disabled)"` diff --git a/pkg/server/server.go b/pkg/server/server.go index 00a065a..4491ae0 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "regexp" "strings" "sync" @@ -33,6 +34,12 @@ type ClickHouseJWEServer struct { // keyed by "host:port". Populated on the first OAuth request to an endpoint; // cleared on config reload so operator changes take effect without restart. chOAuthMethodCache sync.Map + // roleFilterRe is the compiled oauth.role_filter, used to narrow the roles + // activated per ClickHouse request. Compiled up-front in + // NewClickHouseMCPServer when role_claim is set; the roleFilter() getter + // lazily builds it for struct-literal test servers. nil when the feature + // is unconfigured. + roleFilterRe *regexp.Regexp } // ToolHandlerFunc is a function type for tool handlers @@ -75,6 +82,17 @@ func NewClickHouseMCPServer(cfg config.Config, version string) *ClickHouseJWESer oauthVerifier: oauth.NewVerifier(cfg.Server.OAuth), blockedClauses: NormalizeBlockedClauses(cfg.Server.BlockedQueryClauses), } + // Compile the role filter up-front. validateOAuthRuntimeConfig has already + // confirmed it is non-empty and valid when role_claim is set, so a compile + // error here is not expected; leave roleFilterRe nil if it ever occurs so + // role activation fails closed (empty role set => request rejected). + if pat := strings.TrimSpace(cfg.Server.OAuth.RoleFilter); pat != "" { + if re, err := regexp.Compile(pat); err == nil { + chJweServer.roleFilterRe = re + } else { + log.Error().Err(err).Str("role_filter", pat).Msg("oauth: failed to compile role_filter; per-request role activation will fail closed") + } + } // Register tools, resources, and prompts. // Pass pointer to the server's Config so RegisterTools can store converted diff --git a/pkg/server/server_auth_oauth.go b/pkg/server/server_auth_oauth.go index 953e40a..bbac915 100644 --- a/pkg/server/server_auth_oauth.go +++ b/pkg/server/server_auth_oauth.go @@ -3,6 +3,8 @@ package server import ( "context" "net/http" + "regexp" + "strings" "github.com/altinity/go-mcp-oauth-sdk/oauth" ) @@ -107,3 +109,23 @@ func (s *ClickHouseJWEServer) verifier() *oauth.Verifier { } return s.oauthVerifier } + +// roleFilter returns the compiled oauth.role_filter regex, or nil when the +// feature is unconfigured (or the pattern failed to compile, in which case +// callers fail closed). Mirrors verifier(): the constructor path compiles it +// up-front; struct-literal test servers get a lazily-built one. +func (s *ClickHouseJWEServer) roleFilter() *regexp.Regexp { + if s.roleFilterRe != nil { + return s.roleFilterRe + } + pat := strings.TrimSpace(s.Config.Server.OAuth.RoleFilter) + if pat == "" { + return nil + } + re, err := regexp.Compile(pat) + if err != nil { + return nil + } + s.roleFilterRe = re + return re +} diff --git a/pkg/server/server_client.go b/pkg/server/server_client.go index 11a4591..c21f890 100644 --- a/pkg/server/server_client.go +++ b/pkg/server/server_client.go @@ -243,6 +243,21 @@ func (s *ClickHouseJWEServer) GetClickHouseClientWithOAuthForConfig(ctx context. } if s.Config.Server.OAuth.Enabled && oauthToken != "" { + if claimName := strings.TrimSpace(s.Config.Server.OAuth.RoleClaim); claimName != "" { + roles := oauth.RolesFromClaim(oauthClaims, claimName, s.roleFilter()) + if len(roles) == 0 { + // Fail closed: an empty filtered set must NOT fall back to the + // user's default (full) grant. Reject the request. + return nil, fmt.Errorf("oauth: access denied — no ClickHouse roles in claim %q matched role_filter", claimName) + } + if chConfig.Protocol != config.HTTPProtocol { + // role= activation is an HTTP-interface feature; silently + // dropping it on TCP would run the request with the full grant + // (fail open), so refuse instead. + return nil, fmt.Errorf("oauth: role activation requires clickhouse protocol http, got %q", chConfig.Protocol) + } + chConfig.Roles = roles + } return s.newClientWithOAuth(ctx, chConfig, oauthToken) } @@ -271,17 +286,31 @@ func (s *ClickHouseJWEServer) newClientWithOAuth(ctx context.Context, chCfg conf cfg := s.Config.Server.OAuth endpoint := fmt.Sprintf("%s:%d", chCfg.Host, chCfg.Port) - // Cache hit — use the stored method. + // Cache hit — use the stored method. chCfg carries any per-request roles, + // which newClientForOAuthMethod applies via the CH client. if v, ok := s.chOAuthMethodCache.Load(endpoint); ok { return newClientForOAuthMethod(ctx, chCfg, token, v.(chOAuthMethod), cfg) } - // Auto-detect. Try Bearer first. NewClient pings internally, so a failed - // ping surfaces as an auth error here if CH rejects the token. - bearerCfg := oauthApplyBearer(chCfg, token, cfg) + // Auto-detect the CH auth wire format with a ROLE-FREE probe. A role the + // user isn't granted makes CH return ACCESS_DENIED (code 497), which + // isChAuthError treats as an auth failure — on a role-carrying probe that + // would be misread as "Bearer rejected" and trigger a spurious Basic + // fallback. Probing without roles keeps the auth-method signal clean; the + // detected method then builds the real, role-carrying client below. + probeCfg := chCfg + probeCfg.Roles = nil + + // Try Bearer first. NewClient pings internally, so a failed ping surfaces + // as an auth error here if CH rejects the token. + bearerCfg := oauthApplyBearer(probeCfg, token, cfg) if client, err := clickhouse.NewClient(ctx, bearerCfg); err == nil { s.chOAuthMethodCache.Store(endpoint, chOAuthMethodBearer) - return client, nil // return probe client directly — no second ping + if len(chCfg.Roles) == 0 { + return client, nil // no roles — reuse the probe client, no second ping + } + _ = client.Close() + return newClientForOAuthMethod(ctx, chCfg, token, chOAuthMethodBearer, cfg) } else if !isChAuthError(err) { return nil, fmt.Errorf("failed to create ClickHouse client: %w", err) } @@ -292,13 +321,17 @@ func (s *ClickHouseJWEServer) newClientWithOAuth(ctx context.Context, chCfg conf if !ok { return nil, fmt.Errorf("oauth: bearer is not a JWT with an email claim") } - basicCfg := oauthApplyBasic(chCfg, email, token) + basicCfg := oauthApplyBasic(probeCfg, email, token) client, err := clickhouse.NewClient(ctx, basicCfg) if err != nil { return nil, fmt.Errorf("failed to create ClickHouse client: %w", err) } s.chOAuthMethodCache.Store(endpoint, chOAuthMethodBasic) - return client, nil + if len(chCfg.Roles) == 0 { + return client, nil + } + _ = client.Close() + return newClientForOAuthMethod(ctx, chCfg, token, chOAuthMethodBasic, cfg) } // newClientForOAuthMethod applies method to chCfg and creates a CH client. diff --git a/pkg/server/server_client_roles_test.go b/pkg/server/server_client_roles_test.go new file mode 100644 index 0000000..9d4af0f --- /dev/null +++ b/pkg/server/server_client_roles_test.go @@ -0,0 +1,74 @@ +package server + +import ( + "context" + "testing" + + "github.com/altinity/altinity-mcp/pkg/config" + "github.com/stretchr/testify/require" +) + +// roleServer builds a struct-literal server with OAuth + per-request role +// filtering configured. roleFilter() lazily compiles the pattern. +func roleServer(claim, filter string) *ClickHouseJWEServer { + s := &ClickHouseJWEServer{} + s.Config.Server.OAuth.Enabled = true + s.Config.Server.OAuth.RoleClaim = claim + s.Config.Server.OAuth.RoleFilter = filter + return s +} + +const rolesClaimKey = "https://clickhouse/roles" + +// TestOAuthRoleFilterFailClosed verifies that when role_claim is set but no +// role in the claim matches role_filter, the request is rejected before any +// ClickHouse client is built — never falling back to the user's full grant. +func TestOAuthRoleFilterFailClosed(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "_mcp$") + chCfg := config.ClickHouseConfig{Host: "ch.invalid", Port: 8123, Protocol: config.HTTPProtocol} + claims := &OAuthClaims{Extra: map[string]interface{}{ + rolesClaimKey: []interface{}{"admin", "analyst"}, // none end in _mcp + }} + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", "fake-bearer", claims) + require.Error(t, err) + require.Nil(t, client) + require.Contains(t, err.Error(), "access denied") +} + +// TestOAuthRoleFilterRejectsNonHTTP verifies the fail-closed guard for the +// native TCP protocol: role= activation is HTTP-only, so silently running on +// TCP (with the full grant) is refused. +func TestOAuthRoleFilterRejectsNonHTTP(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "_mcp$") + chCfg := config.ClickHouseConfig{Host: "ch.invalid", Port: 9000, Protocol: config.TCPProtocol} + claims := &OAuthClaims{Extra: map[string]interface{}{ + rolesClaimKey: []interface{}{"sandbox_mcp"}, // a match exists + }} + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", "fake-bearer", claims) + require.Error(t, err) + require.Nil(t, client) + require.Contains(t, err.Error(), "protocol http") +} + +// TestOAuthRoleFilterPassesGate verifies that with a matching role over HTTP +// the role checks pass and the code proceeds to client construction — the +// resulting error is a connection failure, not a role/access-denied error. +func TestOAuthRoleFilterPassesGate(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "_mcp$") + // Port 1 refuses immediately, so we exercise the role gate without a 10s dial. + chCfg := config.ClickHouseConfig{Host: "127.0.0.1", Port: 1, Protocol: config.HTTPProtocol} + claims := &OAuthClaims{Extra: map[string]interface{}{ + rolesClaimKey: []interface{}{"sandbox_mcp", "admin"}, + }} + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", "fake-bearer", claims) + require.Error(t, err) + require.Nil(t, client) + require.NotContains(t, err.Error(), "access denied") + require.NotContains(t, err.Error(), "protocol http") +} From 007b0e314c0b255d3d8cc4e9e2bc56fd7ddd44c6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 15 Jun 2026 15:33:40 +0200 Subject: [PATCH 2/5] chore(oauth): switch to published go-mcp-oauth-sdk + document role_claim/role_filter Drop the temporary local replace and pin go-mcp-oauth-sdk at the merged main commit (f4243cf) that carries RoleClaim/RoleFilter + RolesFromClaim. Document the new per-request role-activation config in CLAUDE.md, the helm chart values, and docs/oauth_authorization.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/oauth_authorization.md | 10 ++++++++++ go.mod | 4 +--- go.sum | 4 ++-- helm/altinity-mcp/values.yaml | 10 ++++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/docs/oauth_authorization.md b/docs/oauth_authorization.md index 7f8ed5b..47c76f7 100644 --- a/docs/oauth_authorization.md +++ b/docs/oauth_authorization.md @@ -272,6 +272,14 @@ server: # Scopes required in every incoming bearer JWT required_scopes: [] + # Per-request ClickHouse role activation. role_claim names a JWT claim + # holding a JSON array of role names; only the role_filter-matching subset + # is activated per request (HTTP role= params), narrowing the user's roles. + # role_filter is required when role_claim is set. Empty filtered set fails + # closed (request denied). Leave both empty to disable. + role_claim: "" # e.g. "https://clickhouse/roles" + role_filter: "" # e.g. "_mcp$" + # Token lifetimes (broker mode) access_token_ttl_seconds: 3600 refresh_token_ttl_seconds: 2592000 # 30 d @@ -300,6 +308,8 @@ server: | `public_resource_url` | Externally visible MCP endpoint URL. **Required** behind a reverse proxy. | | `public_auth_server_url` | Externally visible OAuth AS URL. **Required** behind a reverse proxy when `broker: true`. | | `upstream_offline_access` | Request `offline_access` upstream so the IdP consent screen offers long-lived sessions. Default `false`. | +| `role_claim` | JWT claim holding a JSON array of ClickHouse role names to activate per request (e.g. `https://clickhouse/roles`). Empty disables. Read from the validated token's namespaced/custom claims. | +| `role_filter` | Regex selecting which `role_claim` roles are activated (e.g. `_mcp$`). **Required** when `role_claim` is set. Only narrows the user's grant; an empty filtered set fails closed (request denied, no fallback). HTTP protocol only; applies in both Bearer and Basic/sidecar paths. | ## Provider-specific setup diff --git a/go.mod b/go.mod index 95b7583..eba8903 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require golang.org/x/crypto v0.52.0 // indirect require ( github.com/ClickHouse/ch-go v0.71.0 // indirect - github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260527145654-bdefa859fd1b + github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3 github.com/andybalholm/brotli v1.2.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect @@ -45,5 +45,3 @@ require ( golang.org/x/sys v0.45.0 // indirect golang.org/x/text v0.37.0 // indirect ) - -replace github.com/altinity/go-mcp-oauth-sdk => ../go-mcp-oauth-sdk diff --git a/go.sum b/go.sum index fff78ea..2b67c12 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/ClickHouse/ch-go v0.71.0 h1:bUdZ/EZj/LcVHsMqaRUP2holqygrPWQKeMjc6nZoy github.com/ClickHouse/ch-go v0.71.0/go.mod h1:NwbNc+7jaqfY58dmdDUbG4Jl22vThgx1cYjBw0vtgXw= github.com/ClickHouse/clickhouse-go/v2 v2.46.0 h1:s3eRy+hYmu5uzotB6ZhDofgHu8kDgGN/fpmjxRkqSpk= github.com/ClickHouse/clickhouse-go/v2 v2.46.0/go.mod h1:giJfUVlMkcfUEPVfRpt51zZaGEx9i17gCos8gBl392c= -github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260527145654-bdefa859fd1b h1:b6XHGgSAedsoRet5sahr4YABFC2tG+0Imd+8JGx16SY= -github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260527145654-bdefa859fd1b/go.mod h1:yLgv0x586vIzPB5JaA6DkqQsSe4PLRT3PhU2iHO7qsI= +github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3 h1:xiK3uD0pyN4WDNf5XbfTYqVQFxHqnS1ttK5yfgY0JxE= +github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3/go.mod h1:yLgv0x586vIzPB5JaA6DkqQsSe4PLRT3PhU2iHO7qsI= github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= diff --git a/helm/altinity-mcp/values.yaml b/helm/altinity-mcp/values.yaml index 0670eda..42d2954 100644 --- a/helm/altinity-mcp/values.yaml +++ b/helm/altinity-mcp/values.yaml @@ -216,6 +216,16 @@ config: scopes: [] # -- Required scopes for access (token must have all of these) required_scopes: [] + # -- JWT claim holding a JSON array of ClickHouse role names to activate + # per request (e.g. "https://clickhouse/roles"). When set, only the + # role_filter-matching subset is activated via HTTP `role=` params, + # narrowing the user's active roles. Empty disables the feature. + role_claim: "" + # -- Regex selecting which role_claim roles are activated (e.g. "_mcp$"). + # Required when role_claim is set; it is the safety net that stops an + # over-broad claim from activating real-data roles. An empty filtered set + # fails closed (request denied), never falling back to the full grant. + role_filter: "" # Dynamic tools generated from ClickHouse views dynamic_tools: [] # - regexp: "db\\..*" From b598689a084509d4fb6098bd6bd03dc4f336529c Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 15 Jun 2026 16:15:41 +0200 Subject: [PATCH 3/5] fix(oauth): read role claim on the MCP forward path (#141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-request role activation read OAuth claims from context (OAuthClaimsKey), but nothing populates that key on the MCP tools/call path — and ValidateAuth returns nil oauthClaims by design (MCP delegates JWT crypto-validation to ClickHouse, which re-validates the forwarded token per request). So for MCP clients (claude.ai/ChatGPT — exactly what role_claim targets), claims were always nil and the request always failed closed regardless of the token. Fix: when no pre-validated claims are present, decode the role claim from the same token MCP forwards to CH (sharing a new decodeUnverifiedJWTClaims helper refactored out of emailFromUnverifiedJWT). The security boundary is unchanged: the filter only narrows, and CH re-validates the signature and rejects any activated role the user isn't granted (fail-closed). Pre-validated context claims are still preferred when present (e.g. the OpenAPI path). Adds MCP-forward-path tests: matching role passes the gate; no-match fails closed — both with oauthClaims == nil. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/server/server_client.go | 43 ++++++++++++++++++++------ pkg/server/server_client_roles_test.go | 41 ++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/pkg/server/server_client.go b/pkg/server/server_client.go index c21f890..9796af9 100644 --- a/pkg/server/server_client.go +++ b/pkg/server/server_client.go @@ -244,7 +244,18 @@ func (s *ClickHouseJWEServer) GetClickHouseClientWithOAuthForConfig(ctx context. if s.Config.Server.OAuth.Enabled && oauthToken != "" { if claimName := strings.TrimSpace(s.Config.Server.OAuth.RoleClaim); claimName != "" { - roles := oauth.RolesFromClaim(oauthClaims, claimName, s.roleFilter()) + // Prefer pre-validated claims from context; on the MCP forward path + // none are stored (MCP doesn't pre-validate — CH does, per request), + // so fall back to reading the role claim from the same token we + // forward to CH. CH re-validates the signature and rejects any + // activated role the user isn't granted, so this only ever narrows. + claims := oauthClaims + if claims == nil || claims.Extra == nil { + if raw, ok := decodeUnverifiedJWTClaims(oauthToken); ok { + claims = &OAuthClaims{Extra: raw} + } + } + roles := oauth.RolesFromClaim(claims, claimName, s.roleFilter()) if len(roles) == 0 { // Fail closed: an empty filtered set must NOT fall back to the // user's default (full) grant. Reject the request. @@ -387,27 +398,41 @@ func isChAuthError(err error) bool { return auth } -// emailFromUnverifiedJWT decodes the JWT payload without verifying the -// signature and returns the `email` claim (or first namespaced `*/email` -// fallback). Used only to populate the CH `Basic` username so the sidecar can -// receive it; the sidecar still verifies the JWT signature and rejects any -// mismatch between the JWT's signed email and the Basic user. -func emailFromUnverifiedJWT(token string) (string, bool) { +// decodeUnverifiedJWTClaims base64-decodes and JSON-parses a JWT payload +// WITHOUT verifying the signature, returning the raw claim map. MCP does not +// cryptographically validate OAuth JWTs on the forward path — ClickHouse (its +// token_processors / sidecar) re-validates the same forwarded token on every +// request (see ValidateAuth). So this is only used to read claims MCP needs to +// shape the outgoing request, never as a standalone authorization oracle. +func decodeUnverifiedJWTClaims(token string) (map[string]interface{}, bool) { parts := strings.Split(strings.TrimSpace(token), ".") if len(parts) != 3 { - return "", false + return nil, false } payload, err := base64.RawURLEncoding.DecodeString(parts[1]) if err != nil { // Some IdPs emit padded segments; try the std encoding as a fallback. if payload, err = base64.URLEncoding.DecodeString(parts[1]); err != nil { log.Debug().Err(err).Msg("oauth: failed to base64-decode JWT payload") - return "", false + return nil, false } } var raw map[string]interface{} if err := json.Unmarshal(payload, &raw); err != nil { log.Debug().Err(err).Msg("oauth: failed to JSON-parse JWT payload") + return nil, false + } + return raw, true +} + +// emailFromUnverifiedJWT returns the `email` claim (or first namespaced +// `*/email` fallback) from the JWT payload. Used only to populate the CH +// `Basic` username so the sidecar can receive it; the sidecar still verifies +// the JWT signature and rejects any mismatch between the signed email and the +// Basic user. +func emailFromUnverifiedJWT(token string) (string, bool) { + raw, ok := decodeUnverifiedJWTClaims(token) + if !ok { return "", false } if e, ok := raw["email"].(string); ok { diff --git a/pkg/server/server_client_roles_test.go b/pkg/server/server_client_roles_test.go index 9d4af0f..2fbd90c 100644 --- a/pkg/server/server_client_roles_test.go +++ b/pkg/server/server_client_roles_test.go @@ -8,6 +8,10 @@ import ( "github.com/stretchr/testify/require" ) +// fakeJWT (an unsigned header.payload.sig JWT carrying the given claims) is +// defined in server_client_test.go and reused here. MCP doesn't verify the +// signature — CH validates the forwarded token — so the placeholder sig is fine. + // roleServer builds a struct-literal server with OAuth + per-request role // filtering configured. roleFilter() lazily compiles the pattern. func roleServer(claim, filter string) *ClickHouseJWEServer { @@ -72,3 +76,40 @@ func TestOAuthRoleFilterPassesGate(t *testing.T) { require.NotContains(t, err.Error(), "access denied") require.NotContains(t, err.Error(), "protocol http") } + +// TestOAuthRoleFilterReadsClaimFromToken covers the MCP forward path, where no +// pre-validated claims are stored in context (oauthClaims == nil): the role +// claim must be decoded from the forwarded token itself. A matching role passes +// the gate and the request proceeds to client construction (fails only on the +// unreachable host), proving the role was read from the token. +func TestOAuthRoleFilterReadsClaimFromToken(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "^anon_") + chCfg := config.ClickHouseConfig{Host: "127.0.0.1", Port: 1, Protocol: config.HTTPProtocol} + token := fakeJWT(t, map[string]interface{}{ + rolesClaimKey: []interface{}{"anon_reader", "admin_real"}, + }) + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", token, nil) + require.Error(t, err) + require.Nil(t, client) + require.NotContains(t, err.Error(), "access denied") // anon_reader matched ^anon_ + require.NotContains(t, err.Error(), "protocol http") +} + +// TestOAuthRoleFilterFailClosedFromToken is the fail-closed counterpart on the +// MCP forward path: when the token's claim has no role matching the filter, the +// request is denied without building a client. +func TestOAuthRoleFilterFailClosedFromToken(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "^anon_") + chCfg := config.ClickHouseConfig{Host: "127.0.0.1", Port: 1, Protocol: config.HTTPProtocol} + token := fakeJWT(t, map[string]interface{}{ + rolesClaimKey: []interface{}{"admin_real", "analyst"}, + }) + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", token, nil) + require.Error(t, err) + require.Nil(t, client) + require.Contains(t, err.Error(), "access denied") +} From 3f4489a91b5de2cded2b807a123e88ca2e534ec6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 15 Jun 2026 17:08:46 +0200 Subject: [PATCH 4/5] =?UTF-8?q?docs(oauth):=20note=20role=5Ffilter=20is=20?= =?UTF-8?q?a=20partial=20match=20=E2=80=94=20anchor=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit role_filter uses regexp.MatchString (partial match), so an unanchored pattern like "anon" also matches "not_anon_real". Document anchoring (^…/…$) in the helm values and oauth_authorization.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/oauth_authorization.md | 6 ++++-- helm/altinity-mcp/values.yaml | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/oauth_authorization.md b/docs/oauth_authorization.md index 47c76f7..6b3af40 100644 --- a/docs/oauth_authorization.md +++ b/docs/oauth_authorization.md @@ -276,7 +276,9 @@ server: # holding a JSON array of role names; only the role_filter-matching subset # is activated per request (HTTP role= params), narrowing the user's roles. # role_filter is required when role_claim is set. Empty filtered set fails - # closed (request denied). Leave both empty to disable. + # closed (request denied). Leave both empty to disable. Anchor role_filter + # (^…/…$) — it is a partial match, so an unanchored "anon" would also match + # "not_anon_real". role_claim: "" # e.g. "https://clickhouse/roles" role_filter: "" # e.g. "_mcp$" @@ -309,7 +311,7 @@ server: | `public_auth_server_url` | Externally visible OAuth AS URL. **Required** behind a reverse proxy when `broker: true`. | | `upstream_offline_access` | Request `offline_access` upstream so the IdP consent screen offers long-lived sessions. Default `false`. | | `role_claim` | JWT claim holding a JSON array of ClickHouse role names to activate per request (e.g. `https://clickhouse/roles`). Empty disables. Read from the validated token's namespaced/custom claims. | -| `role_filter` | Regex selecting which `role_claim` roles are activated (e.g. `_mcp$`). **Required** when `role_claim` is set. Only narrows the user's grant; an empty filtered set fails closed (request denied, no fallback). HTTP protocol only; applies in both Bearer and Basic/sidecar paths. | +| `role_filter` | Regex selecting which `role_claim` roles are activated (e.g. `_mcp$`). **Required** when `role_claim` is set. Only narrows the user's grant; an empty filtered set fails closed (request denied, no fallback). HTTP protocol only; applies in both Bearer and Basic/sidecar paths. **Anchor it** (`^…`/`…$`) — it's a partial match, so an unanchored `anon` would also match `not_anon_real`. | ## Provider-specific setup diff --git a/helm/altinity-mcp/values.yaml b/helm/altinity-mcp/values.yaml index 42d2954..aa7bbb2 100644 --- a/helm/altinity-mcp/values.yaml +++ b/helm/altinity-mcp/values.yaml @@ -225,6 +225,8 @@ config: # Required when role_claim is set; it is the safety net that stops an # over-broad claim from activating real-data roles. An empty filtered set # fails closed (request denied), never falling back to the full grant. + # Anchor it (^…/…$): it is a partial match, so an unanchored "anon" would + # also match "not_anon_real". role_filter: "" # Dynamic tools generated from ClickHouse views dynamic_tools: [] From e98dfed78c05a95a7162072e34b8bf10940ae4ff Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 15 Jun 2026 19:32:09 +0200 Subject: [PATCH 5/5] feat(oauth): make role_filter optional (empty = activate all claim roles) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IdP is the trust root and ClickHouse independently re-validates the token and enforces grants, so activating exactly the roles the claim carries is safe on its own — filtering is defense-in-depth, not mandatory. Drop the "role_filter required when role_claim is set" rule. - validateOAuthRuntimeConfig: only require role_filter to *compile* when set. - roleFilter(): empty pattern compiles to a match-all regex (activate all). - server constructor: pre-compile the filter whenever role_claim is set. - Unchanged: empty *resolved* set (claim absent/empty, or filter matched nothing) still fails closed — no fallback to the full grant. - Bump go-mcp-oauth-sdk to b30b055 (RoleFilter doc clarified to "optional"). - Docs/helm: role_filter is optional; both modes documented. Tests: config validation (claim w/o filter ok, invalid filter rejected) and server-layer no-filter (activates all claim roles; empty claim still denied). Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/altinity-mcp/main.go | 14 +++++----- cmd/altinity-mcp/main_test.go | 36 +++++++++++++++++++++++++ docs/oauth_authorization.md | 17 ++++++------ go.mod | 2 +- go.sum | 4 +-- helm/altinity-mcp/values.yaml | 12 ++++----- pkg/server/server.go | 12 +++++---- pkg/server/server_auth_oauth.go | 16 +++++------ pkg/server/server_client_roles_test.go | 37 ++++++++++++++++++++++++++ 9 files changed, 111 insertions(+), 39 deletions(-) diff --git a/cmd/altinity-mcp/main.go b/cmd/altinity-mcp/main.go index f895bcc..ccd9df1 100644 --- a/cmd/altinity-mcp/main.go +++ b/cmd/altinity-mcp/main.go @@ -1176,14 +1176,12 @@ func validateOAuthRuntimeConfig(cfg config.Config) error { } } - // Per-request role activation: role_filter is the safety net that stops an - // over-broad/misconfigured role_claim from activating real-data roles, so - // it is required (and must compile) whenever role_claim is set. - if strings.TrimSpace(cfg.Server.OAuth.RoleClaim) != "" { - if strings.TrimSpace(cfg.Server.OAuth.RoleFilter) == "" { - return fmt.Errorf("oauth: role_filter is required when role_claim is set (it bounds which roles may be activated)") - } - if _, err := regexp.Compile(cfg.Server.OAuth.RoleFilter); err != nil { + // Per-request role activation: role_filter is OPTIONAL. When set it narrows + // which role_claim roles are activated and must be a valid regex; when unset + // every role the claim carries is activated (the IdP curates the set, and CH + // re-validates the token and enforces grants). + if rf := strings.TrimSpace(cfg.Server.OAuth.RoleFilter); rf != "" { + if _, err := regexp.Compile(rf); err != nil { return fmt.Errorf("oauth: role_filter is not a valid regex: %w", err) } } diff --git a/cmd/altinity-mcp/main_test.go b/cmd/altinity-mcp/main_test.go index a7c5c28..0ca53d9 100644 --- a/cmd/altinity-mcp/main_test.go +++ b/cmd/altinity-mcp/main_test.go @@ -3409,6 +3409,42 @@ func TestValidateOAuthRuntimeConfig(t *testing.T) { err := validateOAuthRuntimeConfig(cfg) require.ErrorContains(t, err, "token_url") }) + + t.Run("role_claim_without_filter_ok", func(t *testing.T) { + t.Parallel() + o := resourceServerBase() + o.RoleClaim = "https://clickhouse/roles" // role_filter intentionally empty + cfg := config.Config{ + Server: config.ServerConfig{OAuth: o}, + ClickHouse: config.ClickHouseConfig{Protocol: config.HTTPProtocol}, + } + require.NoError(t, validateOAuthRuntimeConfig(cfg)) + }) + + t.Run("role_filter_valid_regex_ok", func(t *testing.T) { + t.Parallel() + o := resourceServerBase() + o.RoleClaim = "https://clickhouse/roles" + o.RoleFilter = "^anon_" + cfg := config.Config{ + Server: config.ServerConfig{OAuth: o}, + ClickHouse: config.ClickHouseConfig{Protocol: config.HTTPProtocol}, + } + require.NoError(t, validateOAuthRuntimeConfig(cfg)) + }) + + t.Run("role_filter_invalid_regex_rejected", func(t *testing.T) { + t.Parallel() + o := resourceServerBase() + o.RoleClaim = "https://clickhouse/roles" + o.RoleFilter = "([" + cfg := config.Config{ + Server: config.ServerConfig{OAuth: o}, + ClickHouse: config.ClickHouseConfig{Protocol: config.HTTPProtocol}, + } + err := validateOAuthRuntimeConfig(cfg) + require.ErrorContains(t, err, "role_filter is not a valid regex") + }) } func TestWarnOAuthMisconfiguration(t *testing.T) { diff --git a/docs/oauth_authorization.md b/docs/oauth_authorization.md index 6b3af40..2a9aa9c 100644 --- a/docs/oauth_authorization.md +++ b/docs/oauth_authorization.md @@ -273,14 +273,15 @@ server: required_scopes: [] # Per-request ClickHouse role activation. role_claim names a JWT claim - # holding a JSON array of role names; only the role_filter-matching subset - # is activated per request (HTTP role= params), narrowing the user's roles. - # role_filter is required when role_claim is set. Empty filtered set fails - # closed (request denied). Leave both empty to disable. Anchor role_filter - # (^…/…$) — it is a partial match, so an unanchored "anon" would also match - # "not_anon_real". + # holding a JSON array of role names, activated per request via HTTP role= + # params. role_filter is OPTIONAL: when set, only the matching subset is + # activated (narrowing); when empty, all roles in the claim are activated. + # Either way an empty resolved set fails closed (request denied, no fallback + # to the full grant). Leave role_claim empty to disable the feature. Anchor + # role_filter (^…/…$) — it is a partial match, so an unanchored "anon" would + # also match "not_anon_real". role_claim: "" # e.g. "https://clickhouse/roles" - role_filter: "" # e.g. "_mcp$" + role_filter: "" # optional; e.g. "^anon_" (empty = activate all claim roles) # Token lifetimes (broker mode) access_token_ttl_seconds: 3600 @@ -311,7 +312,7 @@ server: | `public_auth_server_url` | Externally visible OAuth AS URL. **Required** behind a reverse proxy when `broker: true`. | | `upstream_offline_access` | Request `offline_access` upstream so the IdP consent screen offers long-lived sessions. Default `false`. | | `role_claim` | JWT claim holding a JSON array of ClickHouse role names to activate per request (e.g. `https://clickhouse/roles`). Empty disables. Read from the validated token's namespaced/custom claims. | -| `role_filter` | Regex selecting which `role_claim` roles are activated (e.g. `_mcp$`). **Required** when `role_claim` is set. Only narrows the user's grant; an empty filtered set fails closed (request denied, no fallback). HTTP protocol only; applies in both Bearer and Basic/sidecar paths. **Anchor it** (`^…`/`…$`) — it's a partial match, so an unanchored `anon` would also match `not_anon_real`. | +| `role_filter` | **Optional** regex narrowing which `role_claim` roles are activated (e.g. `_mcp$`). When empty, **all** roles in the claim are activated (the IdP curates the set; CH re-validates the token and enforces grants). Only ever narrows; an empty *resolved* set (claim absent/empty, or filter matched nothing) fails closed — request denied, no fallback to the full grant. HTTP protocol only; applies in both Bearer and Basic/sidecar paths. **Anchor it** (`^…`/`…$`) — it's a partial match, so an unanchored `anon` would also match `not_anon_real`. | ## Provider-specific setup diff --git a/go.mod b/go.mod index eba8903..7fac233 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require golang.org/x/crypto v0.52.0 // indirect require ( github.com/ClickHouse/ch-go v0.71.0 // indirect - github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3 + github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615173123-b30b0552560b github.com/andybalholm/brotli v1.2.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect diff --git a/go.sum b/go.sum index 2b67c12..9e6a941 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/ClickHouse/ch-go v0.71.0 h1:bUdZ/EZj/LcVHsMqaRUP2holqygrPWQKeMjc6nZoy github.com/ClickHouse/ch-go v0.71.0/go.mod h1:NwbNc+7jaqfY58dmdDUbG4Jl22vThgx1cYjBw0vtgXw= github.com/ClickHouse/clickhouse-go/v2 v2.46.0 h1:s3eRy+hYmu5uzotB6ZhDofgHu8kDgGN/fpmjxRkqSpk= github.com/ClickHouse/clickhouse-go/v2 v2.46.0/go.mod h1:giJfUVlMkcfUEPVfRpt51zZaGEx9i17gCos8gBl392c= -github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3 h1:xiK3uD0pyN4WDNf5XbfTYqVQFxHqnS1ttK5yfgY0JxE= -github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615131821-f4243cfb3ce3/go.mod h1:yLgv0x586vIzPB5JaA6DkqQsSe4PLRT3PhU2iHO7qsI= +github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615173123-b30b0552560b h1:2CHdIO/kNhgq077KkhReHLonr/HliNHit8wmKq6wDcU= +github.com/altinity/go-mcp-oauth-sdk v0.1.1-0.20260615173123-b30b0552560b/go.mod h1:yLgv0x586vIzPB5JaA6DkqQsSe4PLRT3PhU2iHO7qsI= github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= diff --git a/helm/altinity-mcp/values.yaml b/helm/altinity-mcp/values.yaml index aa7bbb2..3f4eb8d 100644 --- a/helm/altinity-mcp/values.yaml +++ b/helm/altinity-mcp/values.yaml @@ -221,12 +221,12 @@ config: # role_filter-matching subset is activated via HTTP `role=` params, # narrowing the user's active roles. Empty disables the feature. role_claim: "" - # -- Regex selecting which role_claim roles are activated (e.g. "_mcp$"). - # Required when role_claim is set; it is the safety net that stops an - # over-broad claim from activating real-data roles. An empty filtered set - # fails closed (request denied), never falling back to the full grant. - # Anchor it (^…/…$): it is a partial match, so an unanchored "anon" would - # also match "not_anon_real". + # -- Optional regex narrowing which role_claim roles are activated + # (e.g. "^anon_"). When empty, all roles in the claim are activated (the + # IdP curates the set; CH re-validates the token and enforces grants). An + # empty resolved set fails closed (request denied), never falling back to + # the full grant. Anchor it (^…/…$): it is a partial match, so an + # unanchored "anon" would also match "not_anon_real". role_filter: "" # Dynamic tools generated from ClickHouse views dynamic_tools: [] diff --git a/pkg/server/server.go b/pkg/server/server.go index 4491ae0..e123e8d 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -82,11 +82,13 @@ func NewClickHouseMCPServer(cfg config.Config, version string) *ClickHouseJWESer oauthVerifier: oauth.NewVerifier(cfg.Server.OAuth), blockedClauses: NormalizeBlockedClauses(cfg.Server.BlockedQueryClauses), } - // Compile the role filter up-front. validateOAuthRuntimeConfig has already - // confirmed it is non-empty and valid when role_claim is set, so a compile - // error here is not expected; leave roleFilterRe nil if it ever occurs so - // role activation fails closed (empty role set => request rejected). - if pat := strings.TrimSpace(cfg.Server.OAuth.RoleFilter); pat != "" { + // Compile the role filter up-front when role activation is configured. An + // empty role_filter compiles to a match-all regex (filtering is optional — + // every role_claim role is activated). validateOAuthRuntimeConfig already + // rejected an invalid pattern, so a compile error here is not expected; + // leave roleFilterRe nil if it ever occurs so role activation fails closed. + if strings.TrimSpace(cfg.Server.OAuth.RoleClaim) != "" { + pat := strings.TrimSpace(cfg.Server.OAuth.RoleFilter) if re, err := regexp.Compile(pat); err == nil { chJweServer.roleFilterRe = re } else { diff --git a/pkg/server/server_auth_oauth.go b/pkg/server/server_auth_oauth.go index bbac915..39ba2cf 100644 --- a/pkg/server/server_auth_oauth.go +++ b/pkg/server/server_auth_oauth.go @@ -110,19 +110,17 @@ func (s *ClickHouseJWEServer) verifier() *oauth.Verifier { return s.oauthVerifier } -// roleFilter returns the compiled oauth.role_filter regex, or nil when the -// feature is unconfigured (or the pattern failed to compile, in which case -// callers fail closed). Mirrors verifier(): the constructor path compiles it -// up-front; struct-literal test servers get a lazily-built one. +// roleFilter returns the compiled oauth.role_filter regex. role_filter is +// optional: an empty pattern compiles to a match-all regex, so every role the +// claim carries is activated. Returns nil only if the pattern fails to compile +// (impossible after startup validation), in which case callers fail closed. +// Mirrors verifier(): the constructor compiles it up-front; struct-literal test +// servers get a lazily-built one. func (s *ClickHouseJWEServer) roleFilter() *regexp.Regexp { if s.roleFilterRe != nil { return s.roleFilterRe } - pat := strings.TrimSpace(s.Config.Server.OAuth.RoleFilter) - if pat == "" { - return nil - } - re, err := regexp.Compile(pat) + re, err := regexp.Compile(strings.TrimSpace(s.Config.Server.OAuth.RoleFilter)) if err != nil { return nil } diff --git a/pkg/server/server_client_roles_test.go b/pkg/server/server_client_roles_test.go index 2fbd90c..f6c1c10 100644 --- a/pkg/server/server_client_roles_test.go +++ b/pkg/server/server_client_roles_test.go @@ -113,3 +113,40 @@ func TestOAuthRoleFilterFailClosedFromToken(t *testing.T) { require.Nil(t, client) require.Contains(t, err.Error(), "access denied") } + +// TestOAuthRoleClaimNoFilterActivatesAll verifies that role_filter is optional: +// with role_claim set and no filter, every role the claim carries is activated +// (the IdP curates the set; CH enforces grants). A non-empty set passes the +// gate and proceeds to client construction (failing only on the unreachable +// host). +func TestOAuthRoleClaimNoFilterActivatesAll(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "") // no filter → match all + chCfg := config.ClickHouseConfig{Host: "127.0.0.1", Port: 1, Protocol: config.HTTPProtocol} + token := fakeJWT(t, map[string]interface{}{ + rolesClaimKey: []interface{}{"reader", "writer"}, + }) + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", token, nil) + require.Error(t, err) + require.Nil(t, client) + require.NotContains(t, err.Error(), "access denied") // both roles activated, not denied + require.NotContains(t, err.Error(), "protocol http") +} + +// TestOAuthRoleClaimNoFilterEmptyClaimFailsClosed verifies fail-closed still +// holds without a filter: a token missing the claim yields no roles → denied +// (never falls back to the full grant). +func TestOAuthRoleClaimNoFilterEmptyClaimFailsClosed(t *testing.T) { + t.Parallel() + s := roleServer(rolesClaimKey, "") // no filter + chCfg := config.ClickHouseConfig{Host: "127.0.0.1", Port: 1, Protocol: config.HTTPProtocol} + token := fakeJWT(t, map[string]interface{}{ + "some_other_claim": "x", // role claim absent + }) + + client, err := s.GetClickHouseClientWithOAuthForConfig(context.Background(), chCfg, "", token, nil) + require.Error(t, err) + require.Nil(t, client) + require.Contains(t, err.Error(), "access denied") +}