Skip to content

Commit d404ae5

Browse files
committed
fix(auth): the role picker shows the declared label, not a title-cased name
Deriving the option label by title-casing the machine name made a THIRD source of truth for one string: a position declaring `{ name: 'exec', label: 'Executive' }` rendered as "Exec" in the picker, contradicting the very metadata it came from. Worse for a non-English stack — a position labelled `销售代表` came out as "Sales Rep". Same one-list principle as the role set itself, applied to how it is displayed: `collectStackOrgRoles` carries each entry's declared label through, and `additionalOrgRoles` accepts `{ name, label }` alongside a bare name (a widening — `string[]` callers are unaffected). Title-casing survives only as the fallback for a host that had no label to offer. Presentation only: better-auth is handed names via `orgRoleNames`, and the stored value is always the name. Pinned in the dogfood gate against a real difference — showcase's `exec` declares `Executive`, which title-casing could never produce.
1 parent 072f8a2 commit d404ae5

6 files changed

Lines changed: 173 additions & 52 deletions

File tree

.changeset/app-org-roles-storable.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,19 @@ New exports:
4343

4444
Hosts that boot `AuthPlugin` from a loaded stack should derive
4545
`additionalOrgRoles` with `collectStackOrgRoles(stack)` rather than walking the
46-
stack themselves — `objectstack serve` and the `@objectstack/verify` harness now
47-
both do (the harness previously passed none, which is why a dogfood proof could
48-
boot a stack whose declared roles better-auth had never heard of).
46+
stack themselves — `objectstack serve`, the `@objectstack/verify` harness and
47+
`DevPlugin` now all do. The harness previously passed none, which is why a
48+
dogfood proof could boot a stack whose declared roles better-auth had never
49+
heard of; `DevPlugin` documents itself as equivalent to the full stack and
50+
silently excluded app roles from that equivalence.
51+
52+
`additionalOrgRoles` accepts `{ name, label }` alongside a bare name, and
53+
`collectStackOrgRoles` now returns those descriptors. The label is what the
54+
declaring `position` / `permission` metadata already says, so the role picker
55+
shows `Executive` for a position declared as such instead of title-casing the
56+
machine name into `Exec` — a third source of truth for one string. Presentation
57+
only: better-auth sees just the name, and the stored value is always the name.
58+
Passing `string[]` keeps working unchanged.
4959

5060
Behaviour change worth noting: a declared role name that is not a valid machine
5161
name (`/^[a-z][a-z0-9_]*$/`, min 2 chars) is no longer registered at all, with a

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
2222
import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js';
2323
import { invitationRoleCapFailure, isPlainMemberInvitation } from './invitation-role-cap.js';
24-
import { normalizeAdditionalOrgRoles } from './org-roles.js';
24+
import { normalizeAdditionalOrgRoles, orgRoleNames, type OrgRoleInput } from './org-roles.js';
2525
import { isPlaceholderEmail } from './placeholder-email.js';
2626
import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
2727
import type { TenancyService } from './tenancy-service.js';
@@ -404,12 +404,18 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
404404
* Better-Auth's `member` role) so it cannot inadvertently grant org-level
405405
* admin capabilities.
406406
*
407-
* Typical source: the union of `permission` metadata names that have
408-
* declared names, collected from the loaded stack at CLI boot.
407+
* Typical source: `collectStackOrgRoles(stack)` — the one walk every host
408+
* shares (`objectstack serve`, the verify harness, DevPlugin).
409409
*
410-
* @example ['sales_rep', 'sales_manager', 'service_agent']
410+
* Accepts a bare name, or `{ name, label }` to carry the declaring
411+
* metadata's own display label into the role picker (#3723) — without it the
412+
* picker would title-case the machine name and contradict a position that
413+
* already says `销售代表`. The label is presentation only: better-auth sees
414+
* just the name, and the stored value is always the name.
415+
*
416+
* @example ['sales_rep', { name: 'sales_manager', label: '销售经理' }]
411417
*/
412-
additionalOrgRoles?: string[];
418+
additionalOrgRoles?: OrgRoleInput[];
413419

414420
/**
415421
* Optional outbound email service used by better-auth callbacks
@@ -1601,7 +1607,9 @@ export class AuthManager {
16011607
...stmts,
16021608
invitation: ['create'],
16031609
});
1604-
for (const name of extra) {
1610+
// Names only — a descriptor's `label` is presentation for the role
1611+
// picker and means nothing to better-auth.
1612+
for (const name of orgRoleNames(extra)) {
16051613
if (!name) continue;
16061614
if (built[name]) continue;
16071615
built[name] = defaultAc.newRole(stmts);

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import {
3535
authIdentityObjects,
3636
authPluginManifestHeader,
3737
} from './manifest.js';
38-
import { normalizeAdditionalOrgRoles, withMembershipRoleOptions } from './org-roles.js';
38+
import { normalizeAdditionalOrgRoles, withMembershipRoleOptions, type OrgRoleInput } from './org-roles.js';
3939

4040
/**
4141
* Auth Plugin Options
@@ -80,7 +80,7 @@ export interface AuthPluginOptions extends Partial<AuthConfig> {
8080
* ROLE_NOT_FOUND. Forwarded as-is to AuthManager. See
8181
* {@link AuthManagerOptions.additionalOrgRoles} for details.
8282
*/
83-
additionalOrgRoles?: string[];
83+
additionalOrgRoles?: OrgRoleInput[];
8484

8585
/**
8686
* ADR-0081 D1 — single-org default-organization bootstrap. In single-org

packages/plugins/plugin-auth/src/org-roles.test.ts

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,30 +29,54 @@ const roleOptionsOf = (object: unknown): { label: string; value: string }[] =>
2929
describe('#3723 normalizeAdditionalOrgRoles — the one normalizer', () => {
3030
it('keeps valid app role names, in declaration order', () => {
3131
expect(normalizeAdditionalOrgRoles(['sales_rep', 'sales_manager', 'service_agent'])).toEqual([
32-
'sales_rep',
33-
'sales_manager',
34-
'service_agent',
32+
{ name: 'sales_rep' },
33+
{ name: 'sales_manager' },
34+
{ name: 'service_agent' },
3535
]);
3636
});
3737

38+
it('carries a declared label through, and tolerates entries without one', () => {
39+
expect(
40+
normalizeAdditionalOrgRoles([{ name: 'sales_rep', label: '销售代表' }, 'ops']),
41+
).toEqual([{ name: 'sales_rep', label: '销售代表' }, { name: 'ops' }]);
42+
});
43+
44+
it('drops a blank label rather than storing an empty picker entry', () => {
45+
expect(normalizeAdditionalOrgRoles([{ name: 'ops', label: ' ' }])).toEqual([{ name: 'ops' }]);
46+
});
47+
3848
it('drops the built-ins — an app may not redefine owner/admin/member/delegated_admin', () => {
3949
// Silently downgrading `owner` to a plain-member role would strip its
4050
// `invitation:create` and 403 every org mutation.
4151
expect(
4252
normalizeAdditionalOrgRoles(['owner', 'admin', 'member', 'delegated_admin', 'sales_rep']),
43-
).toEqual(['sales_rep']);
53+
).toEqual([{ name: 'sales_rep' }]);
4454
});
4555

4656
it('de-duplicates and trims', () => {
47-
expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual(['sales_rep']);
57+
expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual([
58+
{ name: 'sales_rep' },
59+
]);
4860
});
4961

50-
it('ignores non-strings and a non-array input', () => {
51-
expect(normalizeAdditionalOrgRoles([null, 42, {}, 'sales_rep'] as unknown[])).toEqual(['sales_rep']);
62+
it('ignores unusable entries and a non-array input', () => {
63+
expect(normalizeAdditionalOrgRoles([null, 42, {}, { name: 7 }, 'sales_rep'] as unknown[])).toEqual([
64+
{ name: 'sales_rep' },
65+
]);
5266
expect(normalizeAdditionalOrgRoles(undefined)).toEqual([]);
5367
expect(normalizeAdditionalOrgRoles(null)).toEqual([]);
5468
});
5569

70+
it('a blank label never masks the built-in drop or the name filter', () => {
71+
expect(
72+
normalizeAdditionalOrgRoles([
73+
{ name: 'owner', label: 'Boss' },
74+
{ name: 'bad.name', label: 'Bad' },
75+
{ name: 'ok_role', label: 'OK' },
76+
]),
77+
).toEqual([{ name: 'ok_role', label: 'OK' }]);
78+
});
79+
5680
it('REFUSES a name that cannot round-trip through Field.select — loudly', () => {
5781
// `Field.select` lowercases values and strips everything outside
5882
// [a-z0-9_], so `showcase.export_data` would be registered with
@@ -64,7 +88,7 @@ describe('#3723 normalizeAdditionalOrgRoles — the one normalizer', () => {
6488
['showcase.export_data', 'Sales Rep', '_leading', 'x', 'sales_rep'],
6589
{ warn },
6690
),
67-
).toEqual(['sales_rep']);
91+
).toEqual([{ name: 'sales_rep' }]);
6892
expect(warn).toHaveBeenCalledTimes(4);
6993
expect(warn.mock.calls.map((c) => String(c[0])).join('\n')).toContain('showcase.export_data');
7094
});
@@ -81,13 +105,13 @@ describe('#3723 membershipRoleOptions — the option list both selects get', ()
81105
});
82106

83107
it('appends app roles after the built-ins, with humanized labels', () => {
84-
const options = membershipRoleOptions(['sales_rep']);
108+
const options = membershipRoleOptions([{ name: 'sales_rep' }]);
85109
expect(values(options)).toEqual(['owner', 'admin', 'delegated_admin', 'member', 'sales_rep']);
86110
expect(options.at(-1)).toEqual({ label: 'Sales Rep', value: 'sales_rep' });
87111
});
88112

89113
it('never duplicates a built-in', () => {
90-
expect(values(membershipRoleOptions(['member', 'sales_rep']))).toEqual([
114+
expect(values(membershipRoleOptions([{ name: 'member' }, { name: 'sales_rep' }]))).toEqual([
91115
'owner',
92116
'admin',
93117
'delegated_admin',
@@ -99,11 +123,20 @@ describe('#3723 membershipRoleOptions — the option list both selects get', ()
99123
it('humanizes labels without ever changing the stored value', () => {
100124
expect(membershipRoleLabel('sales_rep')).toBe('Sales Rep');
101125
expect(membershipRoleLabel('ops')).toBe('Ops');
102-
expect(membershipRoleOptions(['field_ops_delegate']).at(-1)).toEqual({
126+
expect(membershipRoleOptions([{ name: 'field_ops_delegate' }]).at(-1)).toEqual({
103127
label: 'Field Ops Delegate',
104128
value: 'field_ops_delegate',
105129
});
106130
});
131+
132+
it('a declared label WINS over the title-cased machine name', () => {
133+
// The whole point: a position that says `销售代表` must not be rendered as
134+
// "Sales Rep" by a picker deriving its own label from the machine name.
135+
expect(membershipRoleOptions([{ name: 'sales_rep', label: '销售代表' }]).at(-1)).toEqual({
136+
label: '销售代表',
137+
value: 'sales_rep',
138+
});
139+
});
107140
});
108141

109142
describe('#3723 withMembershipRoleOptions — materializing onto the platform objects', () => {
@@ -112,27 +145,27 @@ describe('#3723 withMembershipRoleOptions — materializing onto the platform ob
112145
it('widens BOTH role selects — fixing one leaves the other rejecting the same value', () => {
113146
// The lesson from #3722: `sys_member.role` alone still left
114147
// `sys_invitation.role` refusing the invitation at issuance.
115-
const [, member, invitation] = withMembershipRoleOptions(objects, ['sales_rep']);
148+
const [, member, invitation] = withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]);
116149
expect(values(roleOptionsOf(member))).toContain('sales_rep');
117150
expect(values(roleOptionsOf(invitation))).toContain('sales_rep');
118151
expect(values(roleOptionsOf(member))).toEqual(values(roleOptionsOf(invitation)));
119152
});
120153

121154
it('leaves unrelated objects untouched, by identity', () => {
122-
const [user] = withMembershipRoleOptions(objects, ['sales_rep']);
155+
const [user] = withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]);
123156
expect(user).toBe(SysUser);
124157
});
125158

126159
it('is copy-on-write — the shared module-level definitions are never mutated', () => {
127160
// `authIdentityObjects` is a process-wide singleton also used by the
128161
// compile-time `objectstack.config.ts`; two kernels booted with different
129162
// app roles in one test run must not see each other's options.
130-
withMembershipRoleOptions(objects, ['sales_rep']);
163+
withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]);
131164
expect(values(roleOptionsOf(SysMember))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS));
132165
expect(values(roleOptionsOf(SysInvitation))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS));
133166

134-
const a = withMembershipRoleOptions(objects, ['role_a']);
135-
const b = withMembershipRoleOptions(objects, ['role_b']);
167+
const a = withMembershipRoleOptions(objects, [{ name: 'role_a' }]);
168+
const b = withMembershipRoleOptions(objects, [{ name: 'role_b' }]);
136169
expect(values(roleOptionsOf(a[1]))).toContain('role_a');
137170
expect(values(roleOptionsOf(a[1]))).not.toContain('role_b');
138171
expect(values(roleOptionsOf(b[1]))).toContain('role_b');
@@ -158,20 +191,25 @@ describe('#3723 collectStackOrgRoles — one walk for every host', () => {
158191
positions: [{ name: 'sales_rep' }, { name: 'sales_manager' }],
159192
permissions: [{ name: 'sales_user' }, { name: 'guest_portal' }],
160193
});
161-
expect(roles).toEqual(['sales_rep', 'sales_manager', 'sales_user', 'guest_portal']);
194+
expect(roles.map((r) => r.name)).toEqual([
195+
'sales_rep',
196+
'sales_manager',
197+
'sales_user',
198+
'guest_portal',
199+
]);
162200
});
163201

164202
it('accepts bare strings as well as named entries', () => {
165203
expect(collectStackOrgRoles({ positions: ['sales_rep', { name: 'ops' }] })).toEqual([
166-
'sales_rep',
167-
'ops',
204+
{ name: 'sales_rep' },
205+
{ name: 'ops' },
168206
]);
169207
});
170208

171209
it('normalizes on the way out — the built-ins never leak into the extras', () => {
172210
expect(
173211
collectStackOrgRoles({ positions: [{ name: 'member' }, { name: 'owner' }, { name: 'ops' }] }),
174-
).toEqual(['ops']);
212+
).toEqual([{ name: 'ops' }]);
175213
});
176214

177215
it('a stack with no role metadata yields nothing (and does not throw)', () => {

0 commit comments

Comments
 (0)