feat(cli)!: migrate create rbac bindings to --roleBindings kv syntax
BREAKING: `mcpctl create rbac` no longer accepts `--binding` or
`--operation`. Use `--roleBindings` instead with key:value pairs:
# resource binding
--roleBindings role:view,resource:servers
--roleBindings role:view,resource:servers,name:my-ha
# operation binding (role:run is implied by action:)
--roleBindings action:logs
The on-disk YAML shape (`roleBindings: [{role, resource, name?}]` or
`{role:'run', action}`) is unchanged, so Git backups and existing
`apply -f` files continue to work. Only the command-line input format
changes.
The parser is extracted to src/cli/src/commands/rbac-bindings.ts so the
upcoming `mcpctl create mcptoken --bind <kv>` verb can reuse it.
Completions, tests, and the new parser unit test all pass (406/406).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -194,7 +194,7 @@ _mcpctl() {
|
|||||||
COMPREPLY=($(compgen -W "--description --member --force -h --help" -- "$cur"))
|
COMPREPLY=($(compgen -W "--description --member --force -h --help" -- "$cur"))
|
||||||
;;
|
;;
|
||||||
rbac)
|
rbac)
|
||||||
COMPREPLY=($(compgen -W "--subject --binding --operation --force -h --help" -- "$cur"))
|
COMPREPLY=($(compgen -W "--subject --roleBindings --force -h --help" -- "$cur"))
|
||||||
;;
|
;;
|
||||||
prompt)
|
prompt)
|
||||||
COMPREPLY=($(compgen -W "-p --project --content --content-file --priority --link -h --help" -- "$cur"))
|
COMPREPLY=($(compgen -W "-p --project --content --content-file --priority --link -h --help" -- "$cur"))
|
||||||
|
|||||||
@@ -332,8 +332,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create group" -l force -d 'Update
|
|||||||
|
|
||||||
# create rbac options
|
# create rbac options
|
||||||
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l subject -d 'Subject as Kind:name (repeat for multiple)' -x
|
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l subject -d 'Subject as Kind:name (repeat for multiple)' -x
|
||||||
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l binding -d 'Role binding as role:resource (e.g. edit:servers, run:projects)' -x
|
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l roleBindings -d 'Role binding as key:value pairs, e.g. "role:view,resource:servers" or "role:view,resource:servers,name:my-ha" or "action:logs" (repeat for multiple)' -x
|
||||||
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l operation -d 'Operation binding (e.g. logs, backup)' -x
|
|
||||||
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l force -d 'Update if already exists'
|
complete -c mcpctl -n "__mcpctl_subcmd_active create rbac" -l force -d 'Update if already exists'
|
||||||
|
|
||||||
# create prompt options
|
# create prompt options
|
||||||
|
|||||||
@@ -48,9 +48,39 @@ Before starting the feature, we flushed your in-flight changes to main so they w
|
|||||||
- The mcpd **auth middleware** does not yet dispatch on the token prefix. A raw `mcpctl_pat_…` bearer sent to any `/api/v1/*` endpoint (other than `/introspect`) is still rejected as an invalid session. That's intentional — PR 3 extends `middleware/auth.ts` to recognize both session bearers and McpToken bearers.
|
- The mcpd **auth middleware** does not yet dispatch on the token prefix. A raw `mcpctl_pat_…` bearer sent to any `/api/v1/*` endpoint (other than `/introspect`) is still rejected as an invalid session. That's intentional — PR 3 extends `middleware/auth.ts` to recognize both session bearers and McpToken bearers.
|
||||||
- No CLI yet. Tokens can be created only via `POST /api/v1/mcptokens` for now.
|
- No CLI yet. Tokens can be created only via `POST /api/v1/mcptokens` for now.
|
||||||
|
|
||||||
## PR 2 — RBAC CLI migration
|
## PR 2 — RBAC CLI migration ✅
|
||||||
|
|
||||||
_(blocked by PR 1 — parser is reused by PR 3)_
|
Migrated `mcpctl create rbac` from positional flag syntax to the key=value form you asked for.
|
||||||
|
|
||||||
|
Before:
|
||||||
|
```
|
||||||
|
mcpctl create rbac developers \
|
||||||
|
--subject User:alice@test.com \
|
||||||
|
--binding edit:servers \
|
||||||
|
--binding view:servers:my-ha \
|
||||||
|
--operation logs
|
||||||
|
```
|
||||||
|
After:
|
||||||
|
```
|
||||||
|
mcpctl create rbac developers \
|
||||||
|
--subject User:alice@test.com \
|
||||||
|
--roleBindings role:edit,resource:servers \
|
||||||
|
--roleBindings role:view,resource:servers,name:my-ha \
|
||||||
|
--roleBindings action:logs
|
||||||
|
```
|
||||||
|
|
||||||
|
| # | Step | Status |
|
||||||
|
|---|---|---|
|
||||||
|
| 1 | New shared parser at `src/cli/src/commands/rbac-bindings.ts` exporting `parseRoleBinding(entry)` | ✅ |
|
||||||
|
| 2 | `src/cli/src/commands/create.ts` — old `--binding`/`--operation` flags replaced with one repeatable `--roleBindings <kv>`. Uses the new parser. | ✅ |
|
||||||
|
| 3 | Tests in `src/cli/tests/commands/create.test.ts` rewritten to the new form (8 RBAC tests updated) | ✅ |
|
||||||
|
| 4 | New dedicated unit test `src/cli/tests/commands/rbac-bindings.test.ts` — 9 cases covering unscoped / name-scoped / action / trim / empty-value / unknown-key / action-conflict / missing-role rejections | ✅ |
|
||||||
|
| 5 | Shell completions regenerated via `pnpm completions:generate` — both `completions/mcpctl.{bash,fish}` now offer `--roleBindings`, no longer `--binding`/`--operation` | ✅ |
|
||||||
|
| 6 | Nothing in `docs/` or `README.md` referenced the old flags | ✅ |
|
||||||
|
|
||||||
|
Full CLI suite still 406/406 green. On-disk YAML shape (`roleBindings: [...]`) is unchanged, so backups and existing `apply -f` files keep working.
|
||||||
|
|
||||||
|
The extracted `parseRoleBinding` helper is what PR 3's `mcpctl create mcptoken --bind <kv>` flag will reuse.
|
||||||
|
|
||||||
## PR 3 — CLI mcptoken verbs + mcpd auth dispatch + audit
|
## PR 3 — CLI mcptoken verbs + mcpd auth dispatch + audit
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { Command } from 'commander';
|
import { Command } from 'commander';
|
||||||
import { type ApiClient, ApiError } from '../api-client.js';
|
import { type ApiClient, ApiError } from '../api-client.js';
|
||||||
import { resolveNameOrId } from './shared.js';
|
import { resolveNameOrId } from './shared.js';
|
||||||
|
import { parseRoleBinding } from './rbac-bindings.js';
|
||||||
export interface CreateCommandDeps {
|
export interface CreateCommandDeps {
|
||||||
client: ApiClient;
|
client: ApiClient;
|
||||||
log: (...args: unknown[]) => void;
|
log: (...args: unknown[]) => void;
|
||||||
@@ -331,8 +332,12 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
.description('Create an RBAC binding definition')
|
.description('Create an RBAC binding definition')
|
||||||
.argument('<name>', 'RBAC binding name')
|
.argument('<name>', 'RBAC binding name')
|
||||||
.option('--subject <entry>', 'Subject as Kind:name (repeat for multiple)', collect, [])
|
.option('--subject <entry>', 'Subject as Kind:name (repeat for multiple)', collect, [])
|
||||||
.option('--binding <entry>', 'Role binding as role:resource (e.g. edit:servers, run:projects)', collect, [])
|
.option(
|
||||||
.option('--operation <action>', 'Operation binding (e.g. logs, backup)', collect, [])
|
'--roleBindings <entry>',
|
||||||
|
'Role binding as key:value pairs, e.g. "role:view,resource:servers" or "role:view,resource:servers,name:my-ha" or "action:logs" (repeat for multiple)',
|
||||||
|
collect,
|
||||||
|
[],
|
||||||
|
)
|
||||||
.option('--force', 'Update if already exists')
|
.option('--force', 'Update if already exists')
|
||||||
.action(async (name: string, opts) => {
|
.action(async (name: string, opts) => {
|
||||||
const subjects = (opts.subject as string[]).map((entry: string) => {
|
const subjects = (opts.subject as string[]).map((entry: string) => {
|
||||||
@@ -343,24 +348,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
return { kind: entry.slice(0, colonIdx), name: entry.slice(colonIdx + 1) };
|
return { kind: entry.slice(0, colonIdx), name: entry.slice(colonIdx + 1) };
|
||||||
});
|
});
|
||||||
|
|
||||||
const roleBindings: Array<Record<string, string>> = [];
|
const roleBindings = (opts.roleBindings as string[]).map((entry: string) => parseRoleBinding(entry));
|
||||||
|
|
||||||
// Resource bindings from --binding flag (role:resource or role:resource:name)
|
|
||||||
for (const entry of opts.binding as string[]) {
|
|
||||||
const parts = entry.split(':');
|
|
||||||
if (parts.length === 2) {
|
|
||||||
roleBindings.push({ role: parts[0]!, resource: parts[1]! });
|
|
||||||
} else if (parts.length === 3) {
|
|
||||||
roleBindings.push({ role: parts[0]!, resource: parts[1]!, name: parts[2]! });
|
|
||||||
} else {
|
|
||||||
throw new Error(`Invalid binding format '${entry}'. Expected role:resource or role:resource:name (e.g. edit:servers, view:servers:my-ha)`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Operation bindings from --operation flag
|
|
||||||
for (const action of opts.operation as string[]) {
|
|
||||||
roleBindings.push({ role: 'run', action });
|
|
||||||
}
|
|
||||||
|
|
||||||
const body: Record<string, unknown> = {
|
const body: Record<string, unknown> = {
|
||||||
name,
|
name,
|
||||||
|
|||||||
49
src/cli/src/commands/rbac-bindings.ts
Normal file
49
src/cli/src/commands/rbac-bindings.ts
Normal file
@@ -0,0 +1,49 @@
|
|||||||
|
/**
|
||||||
|
* Parse one `--roleBindings <kv>` entry into a role-binding object the API accepts.
|
||||||
|
*
|
||||||
|
* Accepted forms:
|
||||||
|
* role:view,resource:servers → resource binding (unscoped)
|
||||||
|
* role:view,resource:servers,name:my-ha → resource binding (name-scoped)
|
||||||
|
* action:logs → operation binding (role:run is implied)
|
||||||
|
*
|
||||||
|
* Whitespace around keys/values is trimmed. Keys must be one of: role, resource, name, action.
|
||||||
|
*/
|
||||||
|
export type RoleBindingEntry =
|
||||||
|
| { role: string; resource: string; name?: string }
|
||||||
|
| { role: 'run'; action: string };
|
||||||
|
|
||||||
|
export function parseRoleBinding(entry: string): RoleBindingEntry {
|
||||||
|
const pairs: Record<string, string> = {};
|
||||||
|
for (const part of entry.split(',')) {
|
||||||
|
const colonIdx = part.indexOf(':');
|
||||||
|
if (colonIdx === -1) {
|
||||||
|
throw new Error(`Invalid roleBindings entry '${entry}': expected key:value pairs separated by commas`);
|
||||||
|
}
|
||||||
|
const key = part.slice(0, colonIdx).trim();
|
||||||
|
const value = part.slice(colonIdx + 1).trim();
|
||||||
|
if (!key || !value) {
|
||||||
|
throw new Error(`Invalid roleBindings entry '${entry}': empty key or value`);
|
||||||
|
}
|
||||||
|
if (!['role', 'resource', 'name', 'action'].includes(key)) {
|
||||||
|
throw new Error(`Invalid roleBindings key '${key}' in '${entry}': expected one of role, resource, name, action`);
|
||||||
|
}
|
||||||
|
pairs[key] = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Operation binding: presence of `action:` implies role:run
|
||||||
|
if (pairs['action'] !== undefined) {
|
||||||
|
if (pairs['resource'] !== undefined || pairs['name'] !== undefined) {
|
||||||
|
throw new Error(`Invalid roleBindings entry '${entry}': 'action' cannot be combined with 'resource' or 'name'`);
|
||||||
|
}
|
||||||
|
return { role: 'run', action: pairs['action'] };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Resource binding
|
||||||
|
if (pairs['role'] === undefined || pairs['resource'] === undefined) {
|
||||||
|
throw new Error(`Invalid roleBindings entry '${entry}': need either 'action:…' or both 'role:…,resource:…'`);
|
||||||
|
}
|
||||||
|
if (pairs['name'] !== undefined) {
|
||||||
|
return { role: pairs['role'], resource: pairs['resource'], name: pairs['name'] };
|
||||||
|
}
|
||||||
|
return { role: pairs['role'], resource: pairs['resource'] };
|
||||||
|
}
|
||||||
@@ -318,8 +318,8 @@ describe('create command', () => {
|
|||||||
'rbac', 'developers',
|
'rbac', 'developers',
|
||||||
'--subject', 'User:alice@test.com',
|
'--subject', 'User:alice@test.com',
|
||||||
'--subject', 'Group:dev-team',
|
'--subject', 'Group:dev-team',
|
||||||
'--binding', 'edit:servers',
|
'--roleBindings', 'role:edit,resource:servers',
|
||||||
'--binding', 'view:instances',
|
'--roleBindings', 'role:view,resource:instances',
|
||||||
], { from: 'user' });
|
], { from: 'user' });
|
||||||
|
|
||||||
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
||||||
@@ -342,7 +342,7 @@ describe('create command', () => {
|
|||||||
await cmd.parseAsync([
|
await cmd.parseAsync([
|
||||||
'rbac', 'admins',
|
'rbac', 'admins',
|
||||||
'--subject', 'User:admin@test.com',
|
'--subject', 'User:admin@test.com',
|
||||||
'--binding', 'edit:*',
|
'--roleBindings', 'role:edit,resource:*',
|
||||||
], { from: 'user' });
|
], { from: 'user' });
|
||||||
|
|
||||||
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
||||||
@@ -371,18 +371,18 @@ describe('create command', () => {
|
|||||||
).rejects.toThrow('Invalid subject format');
|
).rejects.toThrow('Invalid subject format');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('throws on invalid binding format', async () => {
|
it('throws on invalid roleBindings format', async () => {
|
||||||
const cmd = createCreateCommand({ client, log });
|
const cmd = createCreateCommand({ client, log });
|
||||||
await expect(
|
await expect(
|
||||||
cmd.parseAsync(['rbac', 'bad', '--binding', 'no-colon'], { from: 'user' }),
|
cmd.parseAsync(['rbac', 'bad', '--roleBindings', 'no-colon'], { from: 'user' }),
|
||||||
).rejects.toThrow('Invalid binding format');
|
).rejects.toThrow(/Invalid roleBindings/);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('throws on 409 without --force', async () => {
|
it('throws on 409 without --force', async () => {
|
||||||
vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"RBAC already exists"}'));
|
vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"RBAC already exists"}'));
|
||||||
const cmd = createCreateCommand({ client, log });
|
const cmd = createCreateCommand({ client, log });
|
||||||
await expect(
|
await expect(
|
||||||
cmd.parseAsync(['rbac', 'developers', '--subject', 'User:a@b.com', '--binding', 'edit:servers'], { from: 'user' }),
|
cmd.parseAsync(['rbac', 'developers', '--subject', 'User:a@b.com', '--roleBindings', 'role:edit,resource:servers'], { from: 'user' }),
|
||||||
).rejects.toThrow('API error 409');
|
).rejects.toThrow('API error 409');
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -393,7 +393,7 @@ describe('create command', () => {
|
|||||||
await cmd.parseAsync([
|
await cmd.parseAsync([
|
||||||
'rbac', 'developers',
|
'rbac', 'developers',
|
||||||
'--subject', 'User:new@test.com',
|
'--subject', 'User:new@test.com',
|
||||||
'--binding', 'edit:*',
|
'--roleBindings', 'role:edit,resource:*',
|
||||||
'--force',
|
'--force',
|
||||||
], { from: 'user' });
|
], { from: 'user' });
|
||||||
|
|
||||||
@@ -404,15 +404,15 @@ describe('create command', () => {
|
|||||||
expect(output.join('\n')).toContain("rbac 'developers' updated");
|
expect(output.join('\n')).toContain("rbac 'developers' updated");
|
||||||
});
|
});
|
||||||
|
|
||||||
it('creates an RBAC definition with operation bindings', async () => {
|
it('creates an RBAC definition with operation bindings (action:… shorthand)', async () => {
|
||||||
vi.mocked(client.post).mockResolvedValueOnce({ id: 'rbac-1', name: 'ops' });
|
vi.mocked(client.post).mockResolvedValueOnce({ id: 'rbac-1', name: 'ops' });
|
||||||
const cmd = createCreateCommand({ client, log });
|
const cmd = createCreateCommand({ client, log });
|
||||||
await cmd.parseAsync([
|
await cmd.parseAsync([
|
||||||
'rbac', 'ops',
|
'rbac', 'ops',
|
||||||
'--subject', 'Group:ops-team',
|
'--subject', 'Group:ops-team',
|
||||||
'--binding', 'edit:servers',
|
'--roleBindings', 'role:edit,resource:servers',
|
||||||
'--operation', 'logs',
|
'--roleBindings', 'action:logs',
|
||||||
'--operation', 'backup',
|
'--roleBindings', 'action:backup',
|
||||||
], { from: 'user' });
|
], { from: 'user' });
|
||||||
|
|
||||||
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
||||||
@@ -433,7 +433,7 @@ describe('create command', () => {
|
|||||||
await cmd.parseAsync([
|
await cmd.parseAsync([
|
||||||
'rbac', 'ha-viewer',
|
'rbac', 'ha-viewer',
|
||||||
'--subject', 'User:alice@test.com',
|
'--subject', 'User:alice@test.com',
|
||||||
'--binding', 'view:servers:my-ha',
|
'--roleBindings', 'role:view,resource:servers,name:my-ha',
|
||||||
], { from: 'user' });
|
], { from: 'user' });
|
||||||
|
|
||||||
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
expect(client.post).toHaveBeenCalledWith('/api/v1/rbac', {
|
||||||
|
|||||||
54
src/cli/tests/commands/rbac-bindings.test.ts
Normal file
54
src/cli/tests/commands/rbac-bindings.test.ts
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { parseRoleBinding } from '../../src/commands/rbac-bindings.js';
|
||||||
|
|
||||||
|
describe('parseRoleBinding', () => {
|
||||||
|
it('parses an unscoped resource binding', () => {
|
||||||
|
expect(parseRoleBinding('role:view,resource:servers')).toEqual({
|
||||||
|
role: 'view',
|
||||||
|
resource: 'servers',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses a name-scoped resource binding', () => {
|
||||||
|
expect(parseRoleBinding('role:view,resource:servers,name:my-ha')).toEqual({
|
||||||
|
role: 'view',
|
||||||
|
resource: 'servers',
|
||||||
|
name: 'my-ha',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses an operation binding via the action shorthand', () => {
|
||||||
|
expect(parseRoleBinding('action:logs')).toEqual({
|
||||||
|
role: 'run',
|
||||||
|
action: 'logs',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('trims whitespace around keys and values', () => {
|
||||||
|
expect(parseRoleBinding('role: edit , resource: * ')).toEqual({
|
||||||
|
role: 'edit',
|
||||||
|
resource: '*',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a pair with no colon', () => {
|
||||||
|
expect(() => parseRoleBinding('role=view')).toThrow(/key:value pairs/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects an unknown key', () => {
|
||||||
|
expect(() => parseRoleBinding('role:view,resource:servers,scope:project')).toThrow(/Invalid roleBindings key 'scope'/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects an empty value', () => {
|
||||||
|
expect(() => parseRoleBinding('role:view,resource:')).toThrow(/empty key or value/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects action combined with resource/name', () => {
|
||||||
|
expect(() => parseRoleBinding('action:logs,resource:servers')).toThrow(/cannot be combined/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('requires both role and resource when action is absent', () => {
|
||||||
|
expect(() => parseRoleBinding('role:view')).toThrow(/need either 'action/);
|
||||||
|
expect(() => parseRoleBinding('resource:servers')).toThrow(/need either 'action/);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user