-
Notifications
You must be signed in to change notification settings - Fork 22
Merge IAM policy hierarchy #1113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c45610b
8273c34
b50c44a
f0a02f8
2e28933
f0da4f1
ba9f2b1
ca73e81
d9bcb23
9e0095d
3c7ef8c
22d37b1
3ad32dd
ceabf51
7dae6dd
2496fd0
a3788c4
cb7960f
705d637
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| aff0d7ace4d92612619289b3e75c597269282166 | ||
| eef65672cf65c5585f3c0340cd0f719009acdcf2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import type { CellContext } from '@tanstack/react-table' | ||
|
|
||
| import type { OrganizationRole, ProjectRole, SiloRole } from '@oxide/api' | ||
| import { Badge } from '@oxide/ui' | ||
|
|
||
| type Role = SiloRole | OrganizationRole | ProjectRole | ||
|
|
||
| /** | ||
| * Highlight the "effective" role in green, others gray. | ||
| * | ||
| * Example: User has collab on org and viewer on project. Collab supersedes | ||
| * because it is the "stronger" role, i.e., it strictly includes the perms on | ||
| * viewer. So collab is highlighted as the "effective" role. | ||
| */ | ||
| export const RoleBadgeCell = <TData extends { effectiveRole: Role }>( | ||
| info: CellContext<TData, Role> | ||
| ) => { | ||
| const cellRole = info.getValue() | ||
| if (!cellRole) return null | ||
| const effectiveRole = info.row.original.effectiveRole | ||
| return ( | ||
| <Badge color={effectiveRole === cellRole ? 'default' : 'neutral'}>{cellRole}</Badge> | ||
| ) | ||
| } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all kind of ridiculous, but it lets us do the latter instead of something like the the former: -cell: (info) => <RoleBadge effectiveRole={info.row.original.effectiveRole} role={info.getValue()} />
+cell: RoleBadge |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,26 +5,27 @@ import type { LoaderFunctionArgs } from 'react-router-dom' | |
|
|
||
| import { | ||
| apiQueryClient, | ||
| orgRoleOrder, | ||
| getEffectiveOrgRole, | ||
| setUserRole, | ||
| useApiMutation, | ||
| useApiQueryClient, | ||
| useUserAccessRows, | ||
| useUserRows, | ||
| } from '@oxide/api' | ||
| import type { OrganizationRole, UserAccessRow } from '@oxide/api' | ||
| import type { OrganizationRole, SiloRole } from '@oxide/api' | ||
| import { useApiQuery } from '@oxide/api' | ||
| import { Table, getActionsCol } from '@oxide/table' | ||
| import { | ||
| Access24Icon, | ||
| Badge, | ||
| Button, | ||
| EmptyMessage, | ||
| PageHeader, | ||
| PageTitle, | ||
| TableActions, | ||
| TableEmptyBox, | ||
| } from '@oxide/ui' | ||
| import { groupBy, isTruthy, sortBy } from '@oxide/util' | ||
|
|
||
| import { RoleBadgeCell } from 'app/components/RoleBadgeCell' | ||
| import { OrgAccessAddUserSideModal, OrgAccessEditUserSideModal } from 'app/forms/org-access' | ||
| import { requireOrgParams, useRequiredParams } from 'app/hooks' | ||
|
|
||
|
|
@@ -42,23 +43,55 @@ const EmptyState = ({ onClick }: { onClick: () => void }) => ( | |
|
|
||
| OrgAccessPage.loader = async ({ params }: LoaderFunctionArgs) => { | ||
| await Promise.all([ | ||
| apiQueryClient.prefetchQuery('policyView', {}), | ||
| apiQueryClient.prefetchQuery('organizationPolicyView', requireOrgParams(params)), | ||
|
Comment on lines
+46
to
47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you demo this, it'd be good to call out that pagination would be a challenge. As I'm thinking about this, it almost seems like these endpoints should more be Again, nothing to do in this PR. |
||
| // used in useUserAccessRows to resolve user names | ||
| // used to resolve user names | ||
| apiQueryClient.prefetchQuery('userList', { limit: 200 }), | ||
| ]) | ||
| } | ||
|
|
||
| type UserRow = UserAccessRow<OrganizationRole> | ||
| type UserRow = { | ||
| id: string | ||
| name: string | ||
| siloRole: SiloRole | undefined | ||
| orgRole: OrganizationRole | undefined | ||
| // all these types are the same but this is strictly more correct than using one | ||
| effectiveRole: SiloRole | OrganizationRole | ||
| } | ||
|
|
||
| const colHelper = createColumnHelper<UserRow>() | ||
|
|
||
| export function OrgAccessPage() { | ||
| const [addModalOpen, setAddModalOpen] = useState(false) | ||
| const [editingUserRow, setEditingUserRow] = useState<UserRow | null>(null) | ||
| const orgParams = useRequiredParams('orgName') | ||
| const { data: policy } = useApiQuery('organizationPolicyView', orgParams) | ||
|
|
||
| const rows = useUserAccessRows(policy, orgRoleOrder) | ||
| const { data: siloPolicy } = useApiQuery('policyView', {}) | ||
| const siloRows = useUserRows(siloPolicy?.roleAssignments, 'silo') | ||
|
|
||
| const { data: orgPolicy } = useApiQuery('organizationPolicyView', orgParams) | ||
| const orgRows = useUserRows(orgPolicy?.roleAssignments, 'org') | ||
|
|
||
| const rows = useMemo(() => { | ||
| const users = groupBy(siloRows.concat(orgRows), (u) => u.id).map( | ||
| ([userId, userAssignments]) => { | ||
| const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName | ||
| const orgRole = userAssignments.find((a) => a.roleSource === 'org')?.roleName | ||
|
|
||
| const roles = [siloRole, orgRole].filter(isTruthy) | ||
|
|
||
| return { | ||
| id: userId, | ||
| name: userAssignments[0].name, | ||
| siloRole, | ||
| orgRole, | ||
| // we know there has to be at least one | ||
| effectiveRole: getEffectiveOrgRole(roles)!, | ||
| } | ||
| } | ||
| ) | ||
| return sortBy(users, (u) => u.name) | ||
| }, [siloRows, orgRows]) | ||
|
|
||
| const queryClient = useApiQueryClient() | ||
| const updatePolicy = useApiMutation('organizationPolicyUpdate', { | ||
|
|
@@ -73,14 +106,20 @@ export function OrgAccessPage() { | |
| () => [ | ||
| colHelper.accessor('id', { header: 'ID' }), | ||
| colHelper.accessor('name', { header: 'Name' }), | ||
| colHelper.accessor('roleName', { | ||
| header: 'Role', | ||
| cell: (info) => <Badge color="neutral">{info.getValue()}</Badge>, | ||
| colHelper.accessor('siloRole', { | ||
| header: 'Silo role', | ||
| cell: RoleBadgeCell, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in addition to being very clean, this actually enforces that the row type has |
||
| }), | ||
| colHelper.accessor('orgRole', { | ||
| header: 'Org role', | ||
| cell: RoleBadgeCell, | ||
| }), | ||
| // TODO: tooltips on disabled elements explaining why | ||
| getActionsCol((row: UserRow) => [ | ||
| { | ||
| label: 'Change role', | ||
| onActivate: () => setEditingUserRow(row), | ||
| disabled: !row.orgRole, | ||
| }, | ||
| // TODO: only show if you have permission to do this | ||
| { | ||
|
|
@@ -90,13 +129,14 @@ export function OrgAccessPage() { | |
| updatePolicy.mutate({ | ||
| ...orgParams, | ||
| // we know policy is there, otherwise there's no row to display | ||
| body: setUserRole(row.id, null, policy!), | ||
| body: setUserRole(row.id, null, orgPolicy!), | ||
| }) | ||
| }, | ||
| disabled: !row.orgRole, | ||
| }, | ||
| ]), | ||
| ], | ||
| [policy, orgParams, updatePolicy] | ||
| [orgPolicy, orgParams, updatePolicy] | ||
| ) | ||
|
|
||
| const tableInstance = useReactTable({ | ||
|
|
@@ -116,22 +156,22 @@ export function OrgAccessPage() { | |
| Add user to organization | ||
| </Button> | ||
| </TableActions> | ||
| {policy && ( | ||
| {orgPolicy && ( | ||
| <OrgAccessAddUserSideModal | ||
| isOpen={addModalOpen} | ||
| onDismiss={() => setAddModalOpen(false)} | ||
| onSuccess={() => setAddModalOpen(false)} | ||
| policy={policy} | ||
| policy={orgPolicy} | ||
| /> | ||
| )} | ||
| {policy && editingUserRow && ( | ||
| {orgPolicy && editingUserRow?.orgRole && ( | ||
| <OrgAccessEditUserSideModal | ||
| isOpen={!!editingUserRow} | ||
| onDismiss={() => setEditingUserRow(null)} | ||
| onSuccess={() => setEditingUserRow(null)} | ||
| policy={policy} | ||
| policy={orgPolicy} | ||
| userId={editingUserRow.id} | ||
| initialValues={{ roleName: editingUserRow.roleName }} | ||
| initialValues={{ roleName: editingUserRow.orgRole }} | ||
| /> | ||
| )} | ||
| {rows.length === 0 ? ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,22 +7,37 @@ test('Click through org access page', async ({ page }) => { | |
|
|
||
| const table = page.locator('role=table') | ||
|
|
||
| // page is there, we see user 1 but not 2 | ||
| // page is there, we see user 1 and 2 but not 3 | ||
| await page.click('role=link[name*="Access & IAM"]') | ||
| await expectVisible(page, ['role=heading[name*="Access & IAM"]']) | ||
| await expectRowVisible(table, { ID: 'user-1', Name: 'Hannah Arendt', Role: 'admin' }) | ||
| await expectNotVisible(page, ['role=cell[name="user-2"]']) | ||
| await expectRowVisible(table, { | ||
| ID: 'user-1', | ||
| Name: 'Hannah Arendt', | ||
| 'Silo role': 'admin', | ||
| 'Org role': '', | ||
| }) | ||
| await expectRowVisible(table, { | ||
| ID: 'user-2', | ||
| Name: 'Hans Jonas', | ||
| 'Silo role': '', | ||
| 'Org role': 'viewer', | ||
| }) | ||
|
Comment on lines
+13
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These really are so much better, ha |
||
| await expectNotVisible(page, ['role=cell[name="user-3"]']) | ||
|
|
||
| // Add user 2 as collab | ||
| await page.click('role=button[name="Add user to organization"]') | ||
| await expectVisible(page, ['role=heading[name*="Add user to organization"]']) | ||
|
|
||
| await page.click('role=button[name="User"]') | ||
| // only users not already on the org should be visible | ||
| await expectNotVisible(page, ['role=option[name="Hannah Arendt"]']) | ||
| await expectVisible(page, ['role=option[name="Hans Jonas"]']) | ||
| await expectNotVisible(page, ['role=option[name="Hans Jonas"]']) | ||
| await expectVisible(page, [ | ||
| 'role=option[name="Hannah Arendt"]', | ||
| 'role=option[name="Jacob Klein"]', | ||
| 'role=option[name="Simone de Beauvoir"]', | ||
| ]) | ||
|
|
||
| await page.click('role=option[name="Hans Jonas"]') | ||
| await page.click('role=option[name="Jacob Klein"]') | ||
|
|
||
| await page.click('role=button[name="Role"]') | ||
| await expectVisible(page, [ | ||
|
|
@@ -34,12 +49,17 @@ test('Click through org access page', async ({ page }) => { | |
| await page.click('role=option[name="Collaborator"]') | ||
| await page.click('role=button[name="Add user"]') | ||
|
|
||
| // User 2 shows up in the table | ||
| await expectRowVisible(table, { ID: 'user-2', Name: 'Hans Jonas', Role: 'collaborator' }) | ||
| // User 3 shows up in the table | ||
| await expectRowVisible(table, { | ||
| ID: 'user-3', | ||
| Name: 'Jacob Klein', | ||
| 'Silo role': '', | ||
| 'Org role': 'collaborator', | ||
| }) | ||
|
|
||
| // now change user 2's role from collab to viewer | ||
| // now change user 3's role from collab to viewer | ||
| await page | ||
| .locator('role=row', { hasText: 'user-2' }) | ||
| .locator('role=row', { hasText: 'user-3' }) | ||
| .locator('role=button[name="Row actions"]') | ||
| .click() | ||
| await page.click('role=menuitem[name="Change role"]') | ||
|
|
@@ -51,7 +71,7 @@ test('Click through org access page', async ({ page }) => { | |
| await page.click('role=option[name="Viewer"]') | ||
| await page.click('role=button[name="Update role"]') | ||
|
|
||
| await expectRowVisible(table, { ID: 'user-2', Role: 'viewer' }) | ||
| await expectRowVisible(table, { ID: 'user-3', 'Org role': 'viewer' }) | ||
|
|
||
| // now delete user 2 | ||
| await page | ||
|
|
@@ -61,4 +81,18 @@ test('Click through org access page', async ({ page }) => { | |
| await expectVisible(page, ['role=cell[name=user-2]']) | ||
| await page.click('role=menuitem[name="Delete"]') | ||
| await expectNotVisible(page, ['role=cell[name=user-2]']) | ||
|
|
||
| // now add an org role to user 1, who currently only has silo role | ||
| await page.click('role=button[name="Add user to organization"]') | ||
| await page.click('role=button[name="User"]') | ||
| await page.click('role=option[name="Hannah Arendt"]') | ||
| await page.click('role=button[name="Role"]') | ||
| await page.click('role=option[name="Viewer"]') | ||
| await page.click('role=button[name="Add user"]') | ||
| await expectRowVisible(table, { | ||
| ID: 'user-1', | ||
| Name: 'Hannah Arendt', | ||
| 'Silo role': 'admin', | ||
| 'Org role': 'viewer', | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visually
secondarymight be a better fit here. It definitely can wait until a design pass though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not be the right place for it, but having tooltips on the non-effective roles would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely. Eugene and Ben are thinking about all that.