From 715c6054a9e0b2f3e2bf59a9fe49b2323d7724b3 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 31 Oct 2025 05:37:46 -0700 Subject: [PATCH 01/25] Bump Omicron version and add limited_collaborator role --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 6 +++--- app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/validate.ts | 4 ++-- app/api/roles.spec.ts | 2 +- app/api/roles.ts | 3 ++- app/forms/access-util.tsx | 5 ++++- app/util/access.ts | 1 + mock-api/msw/handlers.ts | 9 ++++++++- mock-api/msw/util.ts | 3 ++- 10 files changed, 25 insertions(+), 12 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 542956ae1b..65554c7f10 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -9617f40aa6f35b58ffd6b4e92c02ba8e7dd95033 +ceaeadcdcd83c3bfa2e22736c0c4803280bef59f diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index 1697a6c0a7..72bbed88ff 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -3297,7 +3297,7 @@ export type ProjectResultsPage = { nextPage?: string | null } -export type ProjectRole = 'admin' | 'collaborator' | 'viewer' +export type ProjectRole = 'admin' | 'collaborator' | 'limited_collaborator' | 'viewer' /** * Describes the assignment of a particular role on a particular resource to a particular identity (user, group, etc.) @@ -3738,7 +3738,7 @@ export type SiloResultsPage = { nextPage?: string | null } -export type SiloRole = 'admin' | 'collaborator' | 'viewer' +export type SiloRole = 'admin' | 'collaborator' | 'limited_collaborator' | 'viewer' /** * Describes the assignment of a particular role on a particular resource to a particular identity (user, group, etc.) @@ -10110,7 +10110,7 @@ export class Api extends HttpClient { }) }, /** - * Fetch system release repository description by version + * Fetch system release repository by version */ systemUpdateRepositoryView: ( { path }: { path: SystemUpdateRepositoryViewPathParams }, diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index c5a25f06b0..dedfb3ec7e 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -9617f40aa6f35b58ffd6b4e92c02ba8e7dd95033 +ceaeadcdcd83c3bfa2e22736c0c4803280bef59f diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index f7a6949eaa..a1af9104e7 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -3056,7 +3056,7 @@ export const ProjectResultsPage = z.preprocess( export const ProjectRole = z.preprocess( processResponseBody, - z.enum(['admin', 'collaborator', 'viewer']) + z.enum(['admin', 'collaborator', 'limited_collaborator', 'viewer']) ) /** @@ -3438,7 +3438,7 @@ export const SiloResultsPage = z.preprocess( export const SiloRole = z.preprocess( processResponseBody, - z.enum(['admin', 'collaborator', 'viewer']) + z.enum(['admin', 'collaborator', 'limited_collaborator', 'viewer']) ) /** diff --git a/app/api/roles.spec.ts b/app/api/roles.spec.ts index 81b6418f4f..e8b44c6381 100644 --- a/app/api/roles.spec.ts +++ b/app/api/roles.spec.ts @@ -157,5 +157,5 @@ test('byGroupThenName sorts as expected', () => { }) test('allRoles', () => { - expect(allRoles).toEqual(['admin', 'collaborator', 'viewer']) + expect(allRoles).toEqual(['admin', 'collaborator', 'limited_collaborator', 'viewer']) }) diff --git a/app/api/roles.ts b/app/api/roles.ts index e5c9125382..8c7467dcd1 100644 --- a/app/api/roles.ts +++ b/app/api/roles.ts @@ -33,7 +33,8 @@ const flatRoles = (roleOrder: Record): RoleKey[] => export const roleOrder: Record = { collaborator: 1, admin: 0, - viewer: 2, + viewer: 3, + limited_collaborator: 2, } /** `roleOrder` record converted to a sorted array of roles. */ diff --git a/app/forms/access-util.tsx b/app/forms/access-util.tsx index cff6f0ee33..26d30fcee1 100644 --- a/app/forms/access-util.tsx +++ b/app/forms/access-util.tsx @@ -27,7 +27,10 @@ export const defaultValues: AddUserValues = { roleName: '', } -export const roleItems = allRoles.map((role) => ({ value: role, label: capitalize(role) })) +export const roleItems = allRoles.map((role) => ({ + value: role, + label: role.split('_').map(capitalize).join(' '), +})) export const actorToItem = (actor: Actor): ListboxItem => ({ value: actor.id, diff --git a/app/util/access.ts b/app/util/access.ts index e70ec007c0..0e7b374cde 100644 --- a/app/util/access.ts +++ b/app/util/access.ts @@ -18,5 +18,6 @@ export const identityTypeLabel: Record = { export const roleColor: Record = { admin: 'default', collaborator: 'purple', + limited_collaborator: 'purple', viewer: 'blue', } diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 0a2c996ec9..f496ac207b 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -22,6 +22,7 @@ import { type AffinityGroupMember, type AntiAffinityGroupMember, type ApiTypes as Api, + type FleetRole, type InstanceDiskAttachment, type SamlIdentityProvider, } from '@oxide/api' @@ -1626,9 +1627,15 @@ export const handlers = makeHandlers({ systemPolicyView({ cookies }) { requireFleetViewer(cookies) + const fleetRoles: FleetRole[] = ['admin', 'collaborator', 'viewer'] const role_assignments = db.roleAssignments .filter((r) => r.resource_type === 'fleet' && r.resource_id === FLEET_ID) - .map((r) => R.pick(r, ['identity_id', 'identity_type', 'role_name'])) + .filter((r) => fleetRoles.includes(r.role_name as FleetRole)) + .map((r) => ({ + identity_id: r.identity_id, + identity_type: r.identity_type, + role_name: r.role_name as FleetRole, + })) return { role_assignments } }, diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index 741d99733a..1ce6acdc18 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -304,7 +304,8 @@ export function currentUser(cookies: Record): Json { // could implement with `takeUntil(allRoles, r => r === role)`, but that is so // much harder to understand const roleOrStronger: Record = { - viewer: ['viewer', 'collaborator', 'admin'], + viewer: ['viewer', 'limited_collaborator', 'collaborator', 'admin'], + limited_collaborator: ['limited_collaborator', 'collaborator', 'admin'], collaborator: ['collaborator', 'admin'], admin: ['admin'], } From 2a6079ff6eaec5afb90de81da4a43bf4e88f2781 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 31 Oct 2025 07:12:14 -0700 Subject: [PATCH 02/25] Add restriction in MSW around VPC creation and add tests --- mock-api/msw/handlers.ts | 3 ++- mock-api/role-assignment.ts | 9 ++++++- test/e2e/vpcs.e2e.ts | 50 ++++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index f496ac207b..b6995b6f22 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -1155,8 +1155,9 @@ export const handlers = makeHandlers({ const vpcs = db.vpcs.filter((v) => v.project_id === project.id) return paginated(query, vpcs) }, - vpcCreate({ body, query }) { + vpcCreate({ body, query, cookies }) { const project = lookup.project(query) + requireRole(cookies, 'project', project.id, 'collaborator') errIfExists(db.vpcs, { name: body.name, project_id: project.id }) const newVpc: Json = { diff --git a/mock-api/role-assignment.ts b/mock-api/role-assignment.ts index 3fcf4c4e79..bc28a950b7 100644 --- a/mock-api/role-assignment.ts +++ b/mock-api/role-assignment.ts @@ -9,7 +9,7 @@ import { FLEET_ID, type IdentityType, type RoleKey } from '@oxide/api' import { project } from './project' import { defaultSilo } from './silo' -import { user1, user3, user5 } from './user' +import { user1, user2, user3, user5 } from './user' import { userGroup2, userGroup3 } from './user-group' // For most other resources, we can store the API types directly in the DB. But @@ -72,4 +72,11 @@ export const roleAssignments: DbRoleAssignment[] = [ identity_type: 'silo_group', role_name: 'viewer', }, + { + resource_type: 'project', + resource_id: project.id, + identity_id: user2.id, // Hans Jonas + identity_type: 'silo_user', + role_name: 'limited_collaborator', + }, ] diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts index fc8aff4c6d..85ab10877c 100644 --- a/test/e2e/vpcs.e2e.ts +++ b/test/e2e/vpcs.e2e.ts @@ -7,7 +7,7 @@ */ import { expect, test } from '@playwright/test' -import { clickRowAction, expectRowVisible, selectOption } from './utils' +import { clickRowAction, expectRowVisible, getPageAsUser, selectOption } from './utils' test('can nav to VpcPage from /', async ({ page }) => { await page.goto('/') @@ -414,3 +414,51 @@ test('internet gateway shows proper list of routes targeting it', async ({ page '/projects/mock-project/vpcs/mock-vpc/routers/mock-custom-router' ) }) + +test('collaborator can create VPC', async ({ browser }) => { + const page = await getPageAsUser(browser, 'Jacob Klein') + await page.goto('/projects/mock-project/vpcs') + + const table = page.getByRole('table') + + // Create a new VPC + await page.getByRole('link', { name: 'New Vpc' }).click() + await page.getByRole('textbox', { name: 'Name', exact: true }).fill('collab-test-vpc') + await page.getByRole('textbox', { name: 'DNS name' }).fill('collab-test-vpc') + await page.getByRole('button', { name: 'Create VPC' }).click() + + // Should succeed and navigate to the VPC detail page + await expect(page.getByRole('heading', { name: 'collab-test-vpc' })).toBeVisible() + await expect(page.getByRole('tab', { name: 'Firewall Rules' })).toBeVisible() + + // Navigate back to VPCs list to verify it was created + const breadcrumbs = page.getByRole('navigation', { name: 'Breadcrumbs' }) + await breadcrumbs.getByRole('link', { name: 'VPCs' }).click() + + await expectRowVisible(table, { + name: 'collab-test-vpc', + 'DNS name': 'collab-test-vpc', + }) +}) + +test('limited collaborator cannot create VPC', async ({ browser }) => { + const page = await getPageAsUser(browser, 'Hans Jonas') + await page.goto('/projects/mock-project/vpcs') + + // Try to create a new VPC + await page.getByRole('link', { name: 'New Vpc' }).click() + await page.getByRole('textbox', { name: 'Name', exact: true }).fill('limited-test-vpc') + await page.getByRole('textbox', { name: 'DNS name' }).fill('limited-test-vpc') + await page.getByRole('button', { name: 'Create VPC' }).click() + + // Expect the action to fail; Limited Collaborator role does not allow VPC creation + await expect(page.getByText('Action not authorized')).toBeVisible() + + // Close the modal + await page.getByRole('button', { name: 'Close' }).click() + await page.getByRole('button', { name: 'Leave form' }).click() + + // Verify we're still on the VPCs list page and the VPC was not created + await expect(page.getByRole('heading', { name: 'VPCs' })).toBeVisible() + await expect(page.getByRole('cell', { name: 'limited-test-vpc' })).toBeHidden() +}) From 1a3d6dc75d4fb6a4540c2d8a44f4931b5d43a7dd Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 31 Oct 2025 09:28:53 -0700 Subject: [PATCH 03/25] Use radio buttons for forms; refactor --- app/forms/access-util.tsx | 51 ++++++++++++++++++++++++++++++++++++ app/forms/project-access.tsx | 18 +++---------- app/forms/silo-access.tsx | 19 +++----------- app/ui/lib/Radio.tsx | 9 ++++--- 4 files changed, 63 insertions(+), 34 deletions(-) diff --git a/app/forms/access-util.tsx b/app/forms/access-util.tsx index 26d30fcee1..143571d9e4 100644 --- a/app/forms/access-util.tsx +++ b/app/forms/access-util.tsx @@ -5,6 +5,8 @@ * * Copyright Oxide Computer Company */ +import type { Control } from 'react-hook-form' + import { allRoles, type Actor, @@ -14,7 +16,9 @@ import { } from '@oxide/api' import { Badge } from '@oxide/design-system/ui' +import { RadioFieldDyn } from '~/components/form/fields/RadioField' import { type ListboxItem } from '~/ui/lib/Listbox' +import { Radio } from '~/ui/lib/Radio' import { capitalize } from '~/util/str' type AddUserValues = { @@ -32,6 +36,23 @@ export const roleItems = allRoles.map((role) => ({ label: role.split('_').map(capitalize).join(' '), })) +// Role descriptions for project-level roles +const projectRoleDescriptions: Record = { + admin: 'Complete control over the project', + collaborator: 'Can manage all resources, including networking', + limited_collaborator: 'Can manage compute resources, can not manage networking', + viewer: 'Can read most resources within the project', +} + +// Role descriptions for silo-level roles +const siloRoleDescriptions: Record = { + admin: 'Superuser for the silo', + collaborator: 'Can create and own projects; grants project admin role on all projects', + limited_collaborator: + 'Can read most resources within the silo; grants limited collaborator role on all projects', + viewer: 'Can read most resources within the silo; grants project viewer role', +} + export const actorToItem = (actor: Actor): ListboxItem => ({ value: actor.id, label: ( @@ -58,3 +79,33 @@ export type EditRoleModalProps = AddRoleModalProps & { identityType: IdentityType defaultValues: { roleName: RoleKey } } + +type RoleRadioFieldProps = { + control: Control | Control<{ roleName: RoleKey }> + scope: 'Silo' | 'Project' +} + +export function RoleRadioField({ control, scope }: RoleRadioFieldProps) { + const roleDescriptions = scope === 'Silo' ? siloRoleDescriptions : projectRoleDescriptions + return ( + } + column + className="mt-2" + > + {allRoles.map((role) => ( + +
+
+ {capitalize(role).replace('_', ' ')} +
+
{roleDescriptions[role]}
+
+ + ))} + + ) +} diff --git a/app/forms/project-access.tsx b/app/forms/project-access.tsx index ae9551cd37..10b720750d 100644 --- a/app/forms/project-access.tsx +++ b/app/forms/project-access.tsx @@ -22,7 +22,7 @@ import { addToast } from '~/stores/toast' import { actorToItem, defaultValues, - roleItems, + RoleRadioField, type AddRoleModalProps, type EditRoleModalProps, } from './access-util' @@ -75,13 +75,7 @@ export function ProjectAccessAddUserSideModal({ onDismiss, policy }: AddRoleModa required control={form.control} /> - + ) } @@ -124,13 +118,7 @@ export function ProjectAccessEditUserSideModal({ submitError={updatePolicy.error} onDismiss={onDismiss} > - + ) } diff --git a/app/forms/silo-access.tsx b/app/forms/silo-access.tsx index 6eb2e2f701..7fb253b6c4 100644 --- a/app/forms/silo-access.tsx +++ b/app/forms/silo-access.tsx @@ -20,7 +20,7 @@ import { SideModalForm } from '~/components/form/SideModalForm' import { actorToItem, defaultValues, - roleItems, + RoleRadioField, type AddRoleModalProps, type EditRoleModalProps, } from './access-util' @@ -69,13 +69,7 @@ export function SiloAccessAddUserSideModal({ onDismiss, policy }: AddRoleModalPr required control={form.control} /> - + ) } @@ -99,7 +93,6 @@ export function SiloAccessEditUserSideModal({ return ( - + ) } diff --git a/app/ui/lib/Radio.tsx b/app/ui/lib/Radio.tsx index 646c8a364c..dc3523567d 100644 --- a/app/ui/lib/Radio.tsx +++ b/app/ui/lib/Radio.tsx @@ -17,7 +17,10 @@ import cn from 'classnames' import type { ComponentProps } from 'react' // input type is fixed to "radio" -export type RadioProps = Omit, 'type'> +export type RadioProps = Omit, 'type'> & { + /** Align radio button with top of content instead of center (useful for multi-line content) */ + alignTop?: boolean +} const fieldStyles = ` peer appearance-none absolute outline-hidden @@ -26,8 +29,8 @@ const fieldStyles = ` disabled:hover:bg-transparent ` -export const Radio = ({ children, className, ...inputProps }: RadioProps) => ( -