Skip to content

Commit a5f31ce

Browse files
committed
roles: scope person picker to user-kind principals only (CL-6664)
The "Assign a role" person picker showed all principals — people, agents, and workflows — in one flat list. This surfaced 49+ duplicate placeholder- named service accounts (e.g. "Dana Reyes Gaj0a5c7 Localhost") in the picker, none scoped to the workbench's actual member roster. Fix by filtering principals to kind === "user" in both the picker select and the assignments table, matching the People section's member list. Remove the now-unused optgroup structure and PRINCIPAL_KIND imports. All 4 tests rewritten to verify user-only filtering.
1 parent 6c04a90 commit a5f31ce

3 files changed

Lines changed: 63 additions & 46 deletions

File tree

‎packages/settings-ui/src/identity.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@
77
// only, never as visible text.
88
//
99
// `PRINCIPAL_KIND_LABEL` and `PRINCIPAL_KIND_ORDER` live here too, shared by
10-
// every picker that lists principals (Grants' target select and filter,
11-
// Roles' assignment select): Grants/Roles assign to people, agents, *and*
12-
// workflows (see `people-section.tsx`'s own header comment), and a picker
13-
// that shows only names with no kind is kind-blind — a workflow's machine
14-
// principal can look identical to a person's account. Every such picker
15-
// must show which kind an option is, not just its name.
10+
// every picker that lists principals (Grants' target select and filter).
11+
// Grants assign to people, agents, *and* workflows. Roles' assignment picker
12+
// is scoped to user-kind principals only (CL-6664): agents/workflows are
13+
// machine identities that belong on separate surfaces.
1614

1715
import { SETTINGS_STRINGS } from "./strings";
1816

‎packages/settings-ui/src/roles-section.tsx‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,7 @@ import {
3232
UnauthenticatedError,
3333
describeQueryError,
3434
} from "@corbits/api-query";
35-
import {
36-
PRINCIPAL_KIND_LABEL,
37-
PRINCIPAL_KIND_ORDER,
38-
principalLabel,
39-
} from "./identity";
35+
import { principalLabel } from "./identity";
4036
import { SETTINGS_STRINGS } from "./strings";
4137
import {
4238
assignRole,
@@ -317,7 +313,13 @@ export function RoleAssignments({
317313
const [principalId, setPrincipalId] = useState("");
318314
const [roleId, setRoleId] = useState("");
319315

320-
const assignments = principals.flatMap((principal) =>
316+
// CL-6664: Scope both picker and assignments to user-kind principals only.
317+
// Agents and workflows are machine identities — the "Person" picker and
318+
// its assignment table should match the People section's member roster,
319+
// not the full tenant-wide principal list.
320+
const people = principals.filter((p) => p.kind === "user");
321+
322+
const assignments = people.flatMap((principal) =>
321323
principal.roles.map((role) => ({ principal, role })),
322324
);
323325

@@ -333,21 +335,11 @@ export function RoleAssignments({
333335
onChange={(event) => setPrincipalId(event.target.value)}
334336
>
335337
<option value="">—</option>
336-
{PRINCIPAL_KIND_ORDER.map((kind) => {
337-
const kindPrincipals = principals.filter(
338-
(principal) => principal.kind === kind,
339-
);
340-
if (kindPrincipals.length === 0) return null;
341-
return (
342-
<optgroup key={kind} label={PRINCIPAL_KIND_LABEL[kind]}>
343-
{kindPrincipals.map((principal) => (
344-
<option key={principal.id} value={principal.id}>
345-
{principalLabel(principal.displayName).label}
346-
</option>
347-
))}
348-
</optgroup>
349-
);
350-
})}
338+
{people.map((principal) => (
339+
<option key={principal.id} value={principal.id}>
340+
{principalLabel(principal.displayName).label}
341+
</option>
342+
))}
351343
</select>
352344
</label>
353345
<label className="settings-form-field">

‎packages/settings-ui/test/roles-section.test.tsx‎

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
// CL-6077: the "Assign a role" person picker previously listed every
2-
// principal — people, agents, and workflows — as flat, indistinguishable
3-
// options. Grouping by kind (optgroup) means a workflow's machine
4-
// principal can never be mistaken for a person's account in the picker.
1+
// CL-6664: the "Assign a role" person picker must show only user-kind
2+
// principals — the same roster as the People section. Agent/workflow
3+
// machine identities are excluded to prevent placeholder-named garbage
4+
// accounts from polluting the picker.
55

66
import { describe, expect, test } from "bun:test";
77
import { renderToStaticMarkup } from "react-dom/server";
88

9-
import { PRINCIPAL_KIND_LABEL, PRINCIPAL_KIND_ORDER } from "../src/identity";
109
import { RoleAssignments } from "../src/roles-section";
1110

1211
const timestamps = {
@@ -32,7 +31,7 @@ function principal(
3231
}
3332

3433
describe("RoleAssignments picker", () => {
35-
test("groups the person select by principal kind, in user/agent/workflow order", () => {
34+
test("shows only user-kind principals, not agents or workflows", () => {
3635
const markup = renderToStaticMarkup(
3736
<RoleAssignments
3837
roles={[
@@ -45,37 +44,65 @@ describe("RoleAssignments picker", () => {
4544
},
4645
]}
4746
principals={[
48-
principal("workflow", "prn_wf", "Nightly digest"),
4947
principal("user", "prn_user", "Alice Anderson"),
5048
principal("agent", "prn_agent", "Research Assistant"),
49+
principal("workflow", "prn_wf", "Nightly digest"),
5150
]}
5251
onAssign={() => undefined}
5352
onUnassign={() => undefined}
5453
/>,
5554
);
5655

57-
const groupOrder = [...markup.matchAll(/<optgroup label="([^"]+)"/g)].map(
58-
(match) => match[1],
59-
);
60-
expect(groupOrder).toEqual(
61-
PRINCIPAL_KIND_ORDER.map((kind) => PRINCIPAL_KIND_LABEL[kind]),
56+
expect(markup).toContain("Alice Anderson");
57+
expect(markup).not.toContain("Research Assistant");
58+
expect(markup).not.toContain("Nightly digest");
59+
expect(markup).not.toContain("<optgroup");
60+
});
61+
62+
test("shows no optgroups (flat list)", () => {
63+
const markup = renderToStaticMarkup(
64+
<RoleAssignments
65+
roles={[]}
66+
principals={[
67+
principal("user", "prn_1", "Alice Anderson"),
68+
principal("user", "prn_2", "Bob Baker"),
69+
]}
70+
onAssign={() => undefined}
71+
onUnassign={() => undefined}
72+
/>,
6273
);
63-
expect(groupOrder).not.toEqual([...PRINCIPAL_KIND_ORDER]);
74+
expect(markup).not.toContain("<optgroup");
75+
expect(markup).toContain("Alice Anderson");
76+
expect(markup).toContain("Bob Baker");
6477
});
6578

66-
test("omits an empty kind group entirely rather than an empty optgroup", () => {
79+
test("excludes agents from the assignments table too", () => {
6780
const markup = renderToStaticMarkup(
6881
<RoleAssignments
6982
roles={[]}
70-
principals={[principal("user", "prn_user", "Alice Anderson")]}
83+
principals={[
84+
principal("user", "prn_user", "Alice Anderson"),
85+
principal("agent", "prn_agent", "Research Assistant"),
86+
]}
7187
onAssign={() => undefined}
7288
onUnassign={() => undefined}
7389
/>,
7490
);
75-
const groupOrder = [...markup.matchAll(/<optgroup label="([^"]+)"/g)].map(
76-
(match) => match[1],
91+
// The assignments table header should exist but have no rows
92+
expect(markup).toContain("No one has been assigned a role yet.");
93+
expect(markup).not.toContain("Research Assistant");
94+
});
95+
96+
test("does not show user kind label for single-kind list", () => {
97+
const markup = renderToStaticMarkup(
98+
<RoleAssignments
99+
roles={[]}
100+
principals={[principal("user", "prn_1", "Alice Anderson")]}
101+
onAssign={() => undefined}
102+
onUnassign={() => undefined}
103+
/>,
77104
);
78-
expect(groupOrder).toEqual([PRINCIPAL_KIND_LABEL.user]);
79-
expect(groupOrder).not.toEqual(["user"]);
105+
expect(markup).toContain("Alice Anderson");
106+
expect(markup).not.toContain("<optgroup");
80107
});
81108
});

0 commit comments

Comments
 (0)