Skip to content

Commit 2593321

Browse files
authored
Merge pull request #76 from VibeEngineeringAI/fix/demo-protected-roles
fix(frontend): reject protected role grants in demo mode
2 parents 613d11c + 25f5b6b commit 2593321

2 files changed

Lines changed: 89 additions & 3 deletions

File tree

‎frontend/src/lib/demo.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,19 @@ export function createDemoClient(): ParseableClient {
464464
const missing = names.filter((name) => !Object.hasOwn(roles, name));
465465
if (missing.length) fail(`Roles do not exist: ${missing.join(', ')}`);
466466
}
467+
// Demo roles can only hold admin, editor, reader, writer or ingestor, so
468+
// the built-in `super-admin` is the only protected role.
469+
function rejectProtectedRoles(names: string[]) {
470+
if (names.includes('super-admin')) fail('Role is protected');
471+
}
472+
// Like the backend, role additions report protected roles before unknown
473+
// ones, and API keys and user creation the other way round. `super-admin`
474+
// exists on the backend but not in the demo's role map.
475+
function assignableRoles(names: string[], { protectedFirst = false } = {}) {
476+
if (protectedFirst) rejectProtectedRoles(names);
477+
knownRoles(names.filter((name) => name !== 'super-admin'));
478+
rejectProtectedRoles(names);
479+
}
467480
function named(name: string, kind: 'user' | 'role') {
468481
const error = validateName(name, kind);
469482
if (error) fail(error);
@@ -613,7 +626,7 @@ export function createDemoClient(): ParseableClient {
613626
await ready();
614627
if (username === 'admin') fail('Cannot call this API for root admin user');
615628
named(username, 'user');
616-
knownRoles(assigned);
629+
assignableRoles(assigned);
617630
if (!assigned.length) fail('User cannot be created without a role');
618631
if (
619632
users.some(
@@ -641,7 +654,7 @@ export function createDemoClient(): ParseableClient {
641654
async addUserRoles(id, assigned) {
642655
await ready();
643656
const user = findUser(id, true);
644-
knownRoles(assigned);
657+
assignableRoles(assigned, { protectedFirst: true });
645658
if (user.oidc && !user.oidc.legacy)
646659
user.oidc.manualRoles = [...new Set([...(user.oidc.manualRoles ?? []), ...assigned])];
647660
else user.assigned = [...new Set([...user.assigned, ...assigned])];
@@ -678,7 +691,7 @@ export function createDemoClient(): ParseableClient {
678691
},
679692
async createApiKey(keyName, assigned, ttlSeconds) {
680693
await ready();
681-
knownRoles(assigned);
694+
assignableRoles(assigned);
682695
if (
683696
ttlSeconds != null &&
684697
(!Number.isInteger(ttlSeconds) || ttlSeconds < 1800 || ttlSeconds > 7776000)

‎frontend/src/lib/demoTeam.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,79 @@ describe('per-client Team demo', () => {
9595
}
9696
expect(await client.listUsers()).toEqual(users);
9797
});
98+
describe.each([
99+
{ assigned: ['super-admin'] },
100+
{ assigned: ['analysts', 'super-admin'] },
101+
{ assigned: ['super-admin', 'analysts'] },
102+
])('protected role grants: $assigned', ({ assigned }) => {
103+
it('rejects API-key creation without changing the key inventory', async () => {
104+
const client = createDemoClient();
105+
const keys = await client.listApiKeys();
106+
await expect(client.createApiKey('protected-key', assigned)).rejects.toMatchObject({
107+
status: 400,
108+
message: 'Role is protected',
109+
});
110+
expect(await client.listApiKeys()).toEqual(keys);
111+
});
112+
it('rejects user creation without changing users', async () => {
113+
const client = createDemoClient();
114+
const users = await client.listUsers();
115+
await expect(client.createUser('protected-user', assigned)).rejects.toMatchObject({
116+
status: 400,
117+
message: 'Role is protected',
118+
});
119+
expect(await client.listUsers()).toEqual(users);
120+
});
121+
it.each(['analyst', 'https://issuer.example/sso-user'])(
122+
'rejects role additions to %s without changing role sources',
123+
async (id) => {
124+
const client = createDemoClient();
125+
const sources = await client.userRoleSources(id);
126+
await expect(client.addUserRoles(id, assigned)).rejects.toMatchObject({
127+
status: 400,
128+
message: 'Role is protected',
129+
});
130+
expect(await client.userRoleSources(id)).toEqual(sources);
131+
},
132+
);
133+
});
134+
it.each(['constructor', 'toString', '__proto__'])(
135+
'rejects the inherited property name %s as an unknown role',
136+
async (name) => {
137+
const client = createDemoClient();
138+
const unknown = { status: 400, message: `Roles do not exist: ${name}` };
139+
await expect(client.createApiKey('inherited-key', [name])).rejects.toMatchObject(unknown);
140+
await expect(client.createUser('inherited-user', [name])).rejects.toMatchObject(unknown);
141+
await expect(client.addUserRoles('analyst', [name])).rejects.toMatchObject(unknown);
142+
},
143+
);
144+
// Like the backend: API keys and user creation report unknown roles first,
145+
// while role additions report protected roles first.
146+
it('orders unknown and protected role errors like the backend', async () => {
147+
const client = createDemoClient();
148+
const assigned = ['nope', 'super-admin'];
149+
const unknown = { status: 400, message: 'Roles do not exist: nope' };
150+
await expect(client.createApiKey('mixed-key', assigned)).rejects.toMatchObject(unknown);
151+
await expect(client.createUser('mixed-user', assigned)).rejects.toMatchObject(unknown);
152+
await expect(client.addUserRoles('analyst', assigned)).rejects.toMatchObject({
153+
status: 400,
154+
message: 'Role is protected',
155+
});
156+
});
157+
it('allows ordinary admin roles for API keys and user grants', async () => {
158+
const client = createDemoClient();
159+
expect((await client.createApiKey('admin-key', ['administrators'])).roles).toEqual([
160+
'administrators',
161+
]);
162+
await client.createUser('admin-user', ['administrators']);
163+
await client.addUserRoles('analyst', ['administrators']);
164+
for (const id of ['admin-user', 'analyst']) {
165+
expect((await client.userRoleSources(id)).roles.administrators).toEqual({
166+
actions: [{ privilege: 'admin' }],
167+
roleType: 'user',
168+
});
169+
}
170+
});
98171
it('preserves provider overlap, removes manual grants and reconciles the fallback', async () => {
99172
const client = createDemoClient();
100173
const oauth = (await client.listUsers()).find((user) => user.method === 'oauth')!;

0 commit comments

Comments
 (0)