From ce13160068d1993f83311a1dd169bd55c4adbaba Mon Sep 17 00:00:00 2001 From: Bartosz Leper Date: Thu, 12 Dec 2024 17:54:56 +0100 Subject: [PATCH 1/2] More tweaks to the role editor UI - Add default trait references to resource access definitions - Remove the role deletion button --- .../src/Roles/RoleEditor/EditorHeader.tsx | 29 +----- .../src/Roles/RoleEditor/RoleEditor.story.tsx | 1 - .../src/Roles/RoleEditor/RoleEditor.tsx | 14 --- .../Roles/RoleEditor/RoleEditorAdapter.tsx | 3 - .../src/Roles/RoleEditor/RoleEditorDialog.tsx | 7 +- .../Roles/RoleEditor/StandardEditor.test.tsx | 99 ++++++++++++++----- .../Roles/RoleEditor/standardmodel.test.ts | 15 ++- .../src/Roles/RoleEditor/standardmodel.ts | 33 +++++-- web/packages/teleport/src/Roles/Roles.tsx | 1 - 9 files changed, 111 insertions(+), 91 deletions(-) diff --git a/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx b/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx index 4eaaea9bc9e27..a453851633c42 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx @@ -18,11 +18,8 @@ import React from 'react'; import { Flex, H2, Indicator, Box, ButtonIcon } from 'design'; -import { HoverTooltip } from 'design/Tooltip'; +import { Cross } from 'design/Icon'; -import { Cross, Trash } from 'design/Icon'; - -import useTeleport from 'teleport/useTeleport'; import { Role } from 'teleport/services/resources'; import { EditorTab, EditorTabs } from './EditorTabs'; @@ -30,7 +27,6 @@ import { EditorTab, EditorTabs } from './EditorTabs'; /** Renders a header button with role name and delete button. */ export const EditorHeader = ({ role = null, - onDelete, selectedEditorTab, onEditorTabChange, isProcessing, @@ -39,7 +35,6 @@ export const EditorHeader = ({ onClose, }: { role?: Role; - onDelete(): void; selectedEditorTab: EditorTab; onEditorTabChange(t: EditorTab): void; isProcessing: boolean; @@ -47,11 +42,8 @@ export const EditorHeader = ({ yamlEditorId: string; onClose(): void; }) => { - const ctx = useTeleport(); const isCreating = !role; - const hasDeleteAccess = ctx.storeUser.getRoleAccess().remove; - return ( @@ -74,25 +66,6 @@ export const EditorHeader = ({ standardEditorId={standardEditorId} yamlEditorId={yamlEditorId} /> - {!isCreating && ( - - - - - - )} ); }; diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx index e6c44ba571593..b5ed8fd50cbd0 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx @@ -286,7 +286,6 @@ export const Dialog: StoryObj = { open={open} onClose={() => setOpen(false)} onSave={async () => setOpen(false)} - onDelete={async () => setOpen(false)} /> ); diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx index 77945701e8c47..9a116f38c2ffc 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx @@ -27,8 +27,6 @@ import { yamlService } from 'teleport/services/yaml'; import { YamlSupportedResourceKind } from 'teleport/services/yaml/types'; import { CaptureEvent, userEventService } from 'teleport/services/userEvent'; -import DeleteRole from '../DeleteRole'; - import { roleEditorModelToRole, newRole, @@ -49,7 +47,6 @@ export type RoleEditorProps = { originalRole?: RoleWithYaml; onCancel?(): void; onSave?(r: Partial): Promise; - onDelete?(): Promise; }; /** @@ -61,7 +58,6 @@ export const RoleEditor = ({ originalRole, onCancel, onSave, - onDelete, }: RoleEditorProps) => { const idPrefix = useId(); // These IDs are needed to connect accessibility attributes between the @@ -90,8 +86,6 @@ export const RoleEditor = ({ standardModel.roleModel.requiresReset ? EditorTab.Yaml : EditorTab.Standard ); - const [deleting, setDeleting] = useState(false); - // Converts YAML representation to a standard editor model. const [parseAttempt, parseYaml] = useAsync(async () => { const parsedRole = await yamlService.parse( @@ -188,7 +182,6 @@ export const RoleEditor = ({ setDeleting(true)} selectedEditorTab={selectedEditorTab} onEditorTabChange={index => onTabChange(index, validator)} isProcessing={isProcessing} @@ -236,13 +229,6 @@ export const RoleEditor = ({ /> )} - {deleting && ( - setDeleting(false)} - onDelete={onDelete} - /> - )} )} diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditorAdapter.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditorAdapter.tsx index 8a5a6e7bfe3d7..3f03f3d269f19 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditorAdapter.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditorAdapter.tsx @@ -51,12 +51,10 @@ import tagpromo from './tagpromo.png'; export function RoleEditorAdapter({ resources, onSave, - onDelete, onCancel, }: { resources: ResourcesState; onSave: (role: Partial) => Promise; - onDelete: () => Promise; onCancel: () => void; }) { const theme = useTheme(); @@ -106,7 +104,6 @@ export function RoleEditorAdapter({ originalRole={convertAttempt.data} onCancel={onCancel} onSave={onSave} - onDelete={onDelete} /> )} diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditorDialog.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditorDialog.tsx index f5c6370820363..74216fcbaeabe 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditorDialog.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditorDialog.tsx @@ -37,13 +37,11 @@ export function RoleEditorDialog({ onClose, resources, onSave, - onDelete, }: { open: boolean; onClose(): void; resources: ResourcesState; onSave(role: Partial): Promise; - onDelete(): Promise; }) { const transitionRef = useRef(); return ( @@ -61,7 +59,6 @@ export function RoleEditorDialog({ transitionState={transitionState} resources={resources} onSave={onSave} - onDelete={onDelete} /> )} @@ -75,9 +72,8 @@ const DialogInternal = forwardRef< transitionState: TransitionStatus; resources: ResourcesState; onSave(role: Partial): Promise; - onDelete(): Promise; } ->(({ onClose, transitionState, resources, onSave, onDelete }, ref) => { +>(({ onClose, transitionState, resources, onSave }, ref) => { return ( fullScreenDialogCss()} @@ -89,7 +85,6 @@ const DialogInternal = forwardRef< diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx index fc8a1574637f0..f1f0644e343be 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx @@ -244,6 +244,10 @@ describe('ServerAccessSpecSection', () => { kind: 'node', labels: [{ name: 'some-key', value: 'some-value' }], logins: [ + expect.objectContaining({ + label: '{{internal.logins}}', + value: '{{internal.logins}}', + }), expect.objectContaining({ label: 'root', value: 'root' }), expect.objectContaining({ label: 'some-user', value: 'some-user' }), ], @@ -317,6 +321,7 @@ describe('KubernetesAccessSpecSection', () => { expect(onChange).toHaveBeenLastCalledWith({ kind: 'kube_cluster', groups: [ + expect.objectContaining({ value: '{{internal.kubernetes_groups}}' }), expect.objectContaining({ value: 'group1' }), expect.objectContaining({ value: 'group2' }), ], @@ -426,61 +431,89 @@ describe('AppAccessSpecSection', () => { return { user: userEvent.setup(), onChange, validator }; }; - const awsRoleArn = () => - within(screen.getByRole('group', { name: 'AWS Role ARNs' })).getByRole( - 'textbox' - ); - const azureIdentity = () => - within(screen.getByRole('group', { name: 'Azure Identities' })).getByRole( - 'textbox' - ); - const gcpServiceAccount = () => - within( - screen.getByRole('group', { name: 'GCP Service Accounts' }) - ).getByRole('textbox'); + const awsRoleArns = () => + screen.getByRole('group', { name: 'AWS Role ARNs' }); + const awsRoleArnTextBoxes = () => + within(awsRoleArns()).getAllByRole('textbox'); + const azureIdentities = () => + screen.getByRole('group', { name: 'Azure Identities' }); + const azureIdentityTextBoxes = () => + within(azureIdentities()).getAllByRole('textbox'); + const gcpServiceAccounts = () => + screen.getByRole('group', { name: 'GCP Service Accounts' }); + const gcpServiceAccountTextBoxes = () => + within(gcpServiceAccounts()).getAllByRole('textbox'); test('editing', async () => { const { user, onChange } = setup(); await user.click(screen.getByRole('button', { name: 'Add a Label' })); await user.type(screen.getByPlaceholderText('label key'), 'env'); await user.type(screen.getByPlaceholderText('label value'), 'prod'); - await user.type(awsRoleArn(), 'arn:aws:iam::123456789012:role/admin'); + await user.click( + within(awsRoleArns()).getByRole('button', { name: 'Add More' }) + ); + await user.type( + awsRoleArnTextBoxes()[1], + 'arn:aws:iam::123456789012:role/admin' + ); + await user.click( + within(azureIdentities()).getByRole('button', { name: 'Add More' }) + ); await user.type( - azureIdentity(), + azureIdentityTextBoxes()[1], '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin' ); + await user.click( + within(gcpServiceAccounts()).getByRole('button', { name: 'Add More' }) + ); await user.type( - gcpServiceAccount(), + gcpServiceAccountTextBoxes()[1], 'admin@some-project.iam.gserviceaccount.com' ); expect(onChange).toHaveBeenLastCalledWith({ kind: 'app', labels: [{ name: 'env', value: 'prod' }], - awsRoleARNs: ['arn:aws:iam::123456789012:role/admin'], + awsRoleARNs: [ + '{{internal.aws_role_arns}}', + 'arn:aws:iam::123456789012:role/admin', + ], azureIdentities: [ + '{{internal.azure_identities}}', '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin', ], - gcpServiceAccounts: ['admin@some-project.iam.gserviceaccount.com'], + gcpServiceAccounts: [ + '{{internal.gcp_service_accounts}}', + 'admin@some-project.iam.gserviceaccount.com', + ], } as AppAccessSpec); }); test('validation', async () => { const { user, validator } = setup(); await user.click(screen.getByRole('button', { name: 'Add a Label' })); - await user.type(awsRoleArn(), '*'); - await user.type(azureIdentity(), '*'); - await user.type(gcpServiceAccount(), '*'); + await user.click( + within(awsRoleArns()).getByRole('button', { name: 'Add More' }) + ); + await user.type(awsRoleArnTextBoxes()[1], '*'); + await user.click( + within(azureIdentities()).getByRole('button', { name: 'Add More' }) + ); + await user.type(azureIdentityTextBoxes()[1], '*'); + await user.click( + within(gcpServiceAccounts()).getByRole('button', { name: 'Add More' }) + ); + await user.type(gcpServiceAccountTextBoxes()[1], '*'); act(() => validator.validate()); expect( screen.getByPlaceholderText('label key') ).toHaveAccessibleDescription('required'); - expect(awsRoleArn()).toHaveAccessibleDescription( + expect(awsRoleArnTextBoxes()[1]).toHaveAccessibleDescription( 'Wildcard is not allowed in AWS role ARNs' ); - expect(azureIdentity()).toHaveAccessibleDescription( + expect(azureIdentityTextBoxes()[1]).toHaveAccessibleDescription( 'Wildcard is not allowed in Azure identities' ); - expect(gcpServiceAccount()).toHaveAccessibleDescription( + expect(gcpServiceAccountTextBoxes()[1]).toHaveAccessibleDescription( 'Wildcard is not allowed in GCP service accounts' ); }); @@ -521,9 +554,18 @@ describe('DatabaseAccessSpecSection', () => { expect(onChange).toHaveBeenLastCalledWith({ kind: 'db', labels: [{ name: 'env', value: 'prod' }], - names: [expect.objectContaining({ label: 'stuff', value: 'stuff' })], - roles: [expect.objectContaining({ label: 'admin', value: 'admin' })], - users: [expect.objectContaining({ label: 'mary', value: 'mary' })], + names: [ + expect.objectContaining({ value: '{{internal.db_names}}' }), + expect.objectContaining({ label: 'stuff', value: 'stuff' }), + ], + roles: [ + expect.objectContaining({ value: '{{internal.db_roles}}' }), + expect.objectContaining({ label: 'admin', value: 'admin' }), + ], + users: [ + expect.objectContaining({ value: '{{internal.db_users}}' }), + expect.objectContaining({ label: 'mary', value: 'mary' }), + ], } as DatabaseAccessSpec); }); @@ -572,7 +614,10 @@ describe('WindowsDesktopAccessSpecSection', () => { expect(onChange).toHaveBeenLastCalledWith({ kind: 'windows_desktop', labels: [{ name: 'os', value: 'win-xp' }], - logins: [expect.objectContaining({ label: 'julio', value: 'julio' })], + logins: [ + expect.objectContaining({ value: '{{internal.windows_logins}}' }), + expect.objectContaining({ label: 'julio', value: 'julio' }), + ], } as WindowsDesktopAccessSpec); }); diff --git a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts index abf0c76fe9e28..1ce78dc4edf96 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts @@ -32,9 +32,9 @@ import { Label as UILabel } from 'teleport/components/LabelsInput/LabelsInput'; import { Labels } from 'teleport/services/resources'; import { + KubernetesAccessSpec, labelsModelToLabels, labelsToModel, - newAccessSpec, RoleEditorModel, roleEditorModelToRole, roleToRoleEditorModel, @@ -305,6 +305,13 @@ describe('roleToRoleEditorModel', () => { ...minimalRoleModel(), requiresReset: true, }; + // Same as newAccessSpec('kube_cluster'), but without default groups. + const newKubeClusterAccessSpec = (): KubernetesAccessSpec => ({ + kind: 'kube_cluster', + groups: [], + labels: [], + resources: [], + }); test.each<{ name: string; role: Role; model?: RoleEditorModel }>([ { @@ -357,7 +364,7 @@ describe('roleToRoleEditorModel', () => { ...roleModelWithReset, accessSpecs: [ { - ...newAccessSpec('kube_cluster'), + ...newKubeClusterAccessSpec(), resources: [expect.any(Object)], }, ], @@ -383,7 +390,7 @@ describe('roleToRoleEditorModel', () => { ...roleModelWithReset, accessSpecs: [ { - ...newAccessSpec('kube_cluster'), + ...newKubeClusterAccessSpec(), resources: [ expect.objectContaining({ kind: { value: 'job', label: 'Job' } }), ], @@ -413,7 +420,7 @@ describe('roleToRoleEditorModel', () => { ...roleModelWithReset, accessSpecs: [ { - ...newAccessSpec('kube_cluster'), + ...newKubeClusterAccessSpec(), resources: [ expect.objectContaining({ verbs: [{ value: 'get', label: 'get' }], diff --git a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts index 1e5397f0f2556..31d78d67bc56b 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts @@ -353,21 +353,40 @@ export function newAccessSpec(kind: AccessSpecKind): AppAccessSpec; export function newAccessSpec(kind: AccessSpecKind): AccessSpec { switch (kind) { case 'node': - return { kind: 'node', labels: [], logins: [] }; + return { + kind: 'node', + labels: [], + logins: [stringToOption('{{internal.logins}}')], + }; case 'kube_cluster': - return { kind: 'kube_cluster', groups: [], labels: [], resources: [] }; + return { + kind: 'kube_cluster', + groups: [stringToOption('{{internal.kubernetes_groups}}')], + labels: [], + resources: [], + }; case 'app': return { kind: 'app', labels: [], - awsRoleARNs: [], - azureIdentities: [], - gcpServiceAccounts: [], + awsRoleARNs: ['{{internal.aws_role_arns}}'], + azureIdentities: ['{{internal.azure_identities}}'], + gcpServiceAccounts: ['{{internal.gcp_service_accounts}}'], }; case 'db': - return { kind: 'db', labels: [], names: [], users: [], roles: [] }; + return { + kind: 'db', + labels: [], + names: [stringToOption('{{internal.db_names}}')], + users: [stringToOption('{{internal.db_users}}')], + roles: [stringToOption('{{internal.db_roles}}')], + }; case 'windows_desktop': - return { kind: 'windows_desktop', labels: [], logins: [] }; + return { + kind: 'windows_desktop', + labels: [], + logins: [stringToOption('{{internal.windows_logins}}')], + }; default: kind satisfies never; } diff --git a/web/packages/teleport/src/Roles/Roles.tsx b/web/packages/teleport/src/Roles/Roles.tsx index 3437e975bfe5d..b294bfa9b14c5 100644 --- a/web/packages/teleport/src/Roles/Roles.tsx +++ b/web/packages/teleport/src/Roles/Roles.tsx @@ -220,7 +220,6 @@ export function Roles(props: State) { onClose={resources.disregard} resources={resources} onSave={handleSave} - onDelete={handleDelete} /> )} Date: Fri, 13 Dec 2024 17:28:47 +0100 Subject: [PATCH 2/2] Type check fix --- web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx index b5ed8fd50cbd0..ec1aff7c06616 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx @@ -310,7 +310,6 @@ export const DialogWithPolicyEnabled: StoryObj = { open={open} onClose={() => setOpen(false)} onSave={async () => setOpen(false)} - onDelete={async () => setOpen(false)} /> );