docs: document per-caller Kubernetes authorization for the MCP server - #631
Conversation
Five review passes against the actual ske-mcp-server implementation (one cold-reader pass for clarity, four Codex passes for technical accuracy) found and fixed: - The mechanism was explained too abruptly and used RBAC, RoleBinding, and Kubernetes Events undefined; added a numbered sequence and defined each inline. - "Nothing bespoke" and "no configuration required" overstated things given the doc didn't mention prerequisites; added them. - The diagram only showed the RBAC-lookup half of the flow; redrawn to show identity provider through to the granted/denied result. - The new section contradicted the page's older shared-token/ X-User-Identity description with no explanation; added a note reconciling the two. - "Each tool call runs as the caller" overstated scope: only per-namespace request reads/writes are impersonated, not catalog browsing or audit Event writes. Scoped the whole section accordingly. - The RBAC allowlist and identity-prefix prerequisites were understated to the point of being misleading: Kubernetes denies the whole impersonated request if a caller's token carries even one group outside the ServiceAccount's allowlist, not just that group; and an existing RoleBinding on a caller's literal username or group won't match without the configured prefix. Both are now stated directly, since they're the most likely real-world stumbling blocks for a customer setting this up. - Those two prerequisites only apply to human OIDC callers; a client-credentials (machine) caller uses a completely separate, fixed identity mapping and was initially described as if it needed the same configuration. Scoped and documented separately. - Minor: a claimed subject-claim name was overclaimed as "username", a comma splice, RoleBinding-only wording where a ClusterRoleBinding also applies, and a diagram arrow label missing Impersonate-User alongside Impersonate-Group. Corresponds to ske-mcp-server PR #11: five rounds of adversarial review led to real security fixes there too (static-token identity spoofing, streaming buffer leaks, cache-key collisions, actor misclassification, and more) - this section's claims were re-verified against that PR's final state, not an earlier snapshot of it. `yarn build` passes clean.
1e60959 to
fc6e30d
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
jeduardo
left a comment
jeduardo
left a comment
There was a problem hiding this comment.
The prose is very hard to read and looks written as a first pass with the agent. I'm also missing the configuration bits required for this to work, is there anything new needed to make this work?
Also left some comments in the code, but I'd ask the agent to simplify these docs: they look a lot like raw initial agent output and are hard to parse.
|
@jeduardo I have reworked this in 0bbca8d and brought the branch up to current main. The old shared-token customer instructions are now replaced by the production OAuth flow. The deployment section covers the required OIDC claims and scopes, Helm opt-in, release manifests, effective prefixed identities, group allowlisting and customer-authored RoleBindings. Human and machine callers are separate sections, and caveats now has its own heading. One detail from the suggestions is deliberately phrased differently: the server impersonates a caller using its own ServiceAccount credential plus impersonation headers. It never uses the caller's Kubernetes credentials. The page and updated diagram pass |
jeduardo
left a comment
jeduardo
left a comment
There was a problem hiding this comment.
One question and a suggestion, but none are blockers!
3 participants
Summary
X-User-Identitycustomer setup with the current OAuth 2.1 deployment and browser-login flow.MCP_AUTH_TOKENmode.ske-mcp-serverPR https://github.com/syntasso/ske-mcp-server/pull/11 and Helm chart PR feat(ske-mcp-server): support per-caller Kubernetes authorization helm-charts#238.Review feedback addressed
system:authenticated, privileged catalogue and Event operations, and the distinction between Events and Kubernetes Audit logging.Test plan
mainwithout conflicts.yarn install --immutableyarn build