Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions app/forms/project-access.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
updateRole,
useActorsNotInPolicy,
useApiMutation,
type RoleKey,
} from '@oxide/api'
import { Access16Icon } from '@oxide/design-system/icons/react'

Expand Down Expand Up @@ -88,7 +89,11 @@ export function ProjectAccessEditUserSideModal({
identityType,
policy,
defaultValues,
}: EditRoleModalProps) {
}: Omit<EditRoleModalProps, 'defaultValues'> & {
// roleName is optional so the form can open with no role selected for a user
// who has only a silo role today; choosing one adds a new project assignment
defaultValues: { roleName?: RoleKey }
}) {
const { project } = useProjectSelector()

const updatePolicy = useApiMutation(api.projectPolicyUpdate, {
Expand All @@ -106,13 +111,15 @@ export function ProjectAccessEditUserSideModal({
form={form}
formType="edit"
resourceName="role"
title="Edit role"
title={`${defaultValues.roleName ? 'Edit' : 'Add'} project role`}
subtitle={
<ResourceLabel>
<Access16Icon /> {name}
</ResourceLabel>
}
onSubmit={({ roleName }) => {
// required validation prevents submitting without a selection
if (!roleName) return
updatePolicy.mutate({
path: { project },
body: updateRole({ identityId, identityType, roleName }, policy),
Expand Down
35 changes: 28 additions & 7 deletions app/pages/project/access/ProjectAccessPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
roleOrder,
useApiMutation,
usePrefetchedQuery,
userRoleFromPolicies,
useUserRows,
type IdentityType,
type RoleKey,
Expand All @@ -34,6 +35,7 @@ import {
ProjectAccessAddUserSideModal,
ProjectAccessEditUserSideModal,
} from '~/forms/project-access'
import { useCurrentUser } from '~/hooks/use-current-user'
import { getProjectSelector, useProjectSelector } from '~/hooks/use-params'
import { useQuickActions } from '~/hooks/use-quick-actions'
import { confirmDelete } from '~/stores/confirm-delete'
Expand Down Expand Up @@ -103,6 +105,17 @@ export default function ProjectAccessPage() {
const { data: projectPolicy } = usePrefetchedQuery(projectPolicyView(projectSelector))
const projectRows = useUserRows(projectPolicy.roleAssignments, 'project')

// Changing a project role assignment requires `modify` on the project, i.e.
// the project `admin` role. Per the authz policy, that role is conferred
// either by a direct project admin role or by a silo collaborator/admin role
// (a silo collaborator is an admin on every project in the silo).
// https://github.com/oxidecomputer/omicron/blob/main/nexus/auth/src/authz/omicron.polar
const { me, myGroups } = useCurrentUser()
const myProjectRole = userRoleFromPolicies(me, myGroups.items, [projectPolicy])
const mySiloRole = userRoleFromPolicies(me, myGroups.items, [siloPolicy])
const canEditRoles =
myProjectRole === 'admin' || mySiloRole === 'admin' || mySiloRole === 'collaborator'

const rows = useMemo(() => {
return groupBy(siloRows.concat(projectRows), (u) => u.id)
.map(([userId, userAssignments]) => {
Expand Down Expand Up @@ -166,15 +179,16 @@ export default function ProjectAccessPage() {
),
}),

// TODO: tooltips on disabled elements explaining why
getActionsCol((row: UserRow) => [
{
label: 'Change role',
// An admin can change any row's project role, even one that only has
// a silo role, because doing so adds a new project assignment.
label: `${row.projectRole ? 'Change' : 'Add'} project role`,
onActivate: () => setEditingUserRow(row),
disabled:
!row.projectRole && "You don't have permission to change this user's role",
!canEditRoles &&
`You don't have permission to ${row.projectRole ? 'change' : 'add'} project roles`,
},
// TODO: only show if you have permission to do this
{
label: 'Delete',
onActivate: confirmDelete({
Expand All @@ -194,11 +208,17 @@ export default function ProjectAccessPage() {
),
resourceKind: 'role assignment',
}),
disabled: !row.projectRole && "You don't have permission to delete this user",
disabled: !canEditRoles
? "You don't have permission to remove roles"
: // There's no project assignment to delete; the role is inherited
// from the silo and can only be changed on the silo access page.
!row.projectRole
? 'This role is inherited from the silo'
: undefined,
},
]),
],
[projectPolicy, projectSelector.project, updatePolicy]
[canEditRoles, projectPolicy, projectSelector.project, updatePolicy]
)

const tableInstance = useReactTable({
Expand Down Expand Up @@ -239,13 +259,14 @@ export default function ProjectAccessPage() {
policy={projectPolicy}
/>
)}
{projectPolicy && editingUserRow?.projectRole && (
{projectPolicy && editingUserRow && (
<ProjectAccessEditUserSideModal
onDismiss={() => setEditingUserRow(null)}
policy={projectPolicy}
name={editingUserRow.name}
identityId={editingUserRow.id}
identityType={editingUserRow.identityType}
// leave unselected when the user has no project role yet
defaultValues={{ roleName: editingUserRow.projectRole }}
/>
)}
Expand Down
54 changes: 51 additions & 3 deletions test/e2e/project-access.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@
*/
import { user3, user4 } from '@oxide/api-mocks'

import { expect, expectNotVisible, expectRowVisible, expectVisible, test } from './utils'
import {
expect,
expectNotVisible,
expectRowVisible,
expectVisible,
getPageAsUser,
test,
} from './utils'

test('Click through project access page', async ({ page }) => {
await page.goto('/projects/mock-project')
Expand Down Expand Up @@ -47,6 +54,18 @@ test('Click through project access page', async ({ page }) => {
`role=cell[name="Simone de Beauvoir"]`,
])

// Hannah Arendt has only a silo role, so the action reads "Add project role"
// and the form opens with nothing preselected
await page
.getByRole('row', { name: 'Hannah Arendt', exact: false })
.getByRole('button', { name: 'Row actions' })
.click()
await page.click('role=menuitem[name="Add project role"]')
const editDialog = page.getByRole('dialog')
await expect(editDialog.getByRole('heading', { name: /Add project role/ })).toBeVisible()
await expect(editDialog.getByRole('radio', { checked: true })).toHaveCount(0)
await editDialog.getByRole('button', { name: 'Cancel' }).click()

// Add user 4 as collab
await page.click('role=button[name="Add user or group"]')
await expectVisible(page, ['role=heading[name*="Add user or group"]'])
Expand Down Expand Up @@ -80,9 +99,9 @@ test('Click through project access page', async ({ page }) => {
.locator('role=row', { hasText: user4.display_name })
.locator('role=button[name="Row actions"]')
.click()
await page.click('role=menuitem[name="Change role"]')
await page.click('role=menuitem[name="Change project role"]')

await expectVisible(page, ['role=heading[name*="Edit role"]'])
await expectVisible(page, ['role=heading[name*="Edit project role"]'])

// Verify Collaborator is currently selected
await expect(page.getByRole('radio', { name: /^Collaborator / })).toBeChecked()
Expand Down Expand Up @@ -116,3 +135,32 @@ test('Click through project access page', async ({ page }) => {
Role: 'silo.admin+1',
})
})

test('Non-admin cannot change or remove roles', async ({ browser }) => {
// Jacob Klein is only a project collaborator (not project admin or silo
// collaborator/admin), so he lacks `modify` on the project and can't change
// role assignments. Both row actions should be disabled.
const page = await getPageAsUser(browser, 'Jacob Klein')
await page.goto('/projects/mock-project/access')

await expect(page.getByRole('heading', { name: /Access/ })).toBeVisible()

await page
.getByRole('row', { name: 'Herbert Marcuse', exact: false })
.getByRole('button', { name: 'Row actions' })
.click()

const changeRole = page.getByRole('menuitem', { name: 'Change project role' })
await expect(changeRole).toBeDisabled()
await changeRole.hover()
await expect(page.getByRole('tooltip')).toHaveText(
"You don't have permission to change project roles"
)

const deleteItem = page.getByRole('menuitem', { name: 'Delete' })
await expect(deleteItem).toBeDisabled()
await deleteItem.hover()
await expect(page.getByRole('tooltip')).toHaveText(
"You don't have permission to remove roles"
)
})
Loading