diff --git a/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx b/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx index 541e6f08bfefa..2ee25880cedad 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx @@ -17,20 +17,32 @@ */ import React from 'react'; -import { Flex, ButtonText, H2 } from 'design'; +import { Flex, ButtonText, H2, Indicator, Box } from 'design'; import { HoverTooltip } from 'design/Tooltip'; import { Trash } from 'design/Icon'; import useTeleport from 'teleport/useTeleport'; import { Role } from 'teleport/services/resources'; +import { EditorTab, EditorTabs } from './EditorTabs'; + /** Renders a header button with role name and delete button. */ export const EditorHeader = ({ role = null, onDelete, + selectedEditorTab, + onEditorTabChange, + isProcessing, + standardEditorId, + yamlEditorId, }: { - onDelete?(): void; role?: Role; + onDelete(): void; + selectedEditorTab: EditorTab; + onEditorTabChange(t: EditorTab): void; + isProcessing: boolean; + standardEditorId: string; + yamlEditorId: string; }) => { const ctx = useTeleport(); const isCreating = !role; @@ -38,8 +50,24 @@ export const EditorHeader = ({ const hasDeleteAccess = ctx.storeUser.getRoleAccess().remove; return ( - -

{isCreating ? 'Create a New Role' : role?.metadata.name}

+ + +

+ {isCreating + ? 'Create a New Role' + : `Edit Role ${role?.metadata.name}`} +

+ + + {isProcessing && } + + {!isCreating && ( { + const standardLabel = 'Switch to standard editor'; + const yamlLabel = 'Switch to YAML editor'; return ( ); }; diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx index 235a0d83782a6..1376c078e80ff 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx @@ -75,10 +75,7 @@ afterEach(() => { test('rendering and switching tabs for new role', async () => { render(); - expect(screen.getByRole('tab', { name: 'Standard' })).toHaveAttribute( - 'aria-selected', - 'true' - ); + expect(getStandardEditorTab()).toHaveAttribute('aria-selected', 'true'); expect( screen.queryByRole('button', { name: /Reset to Standard Settings/i }) ).not.toBeInTheDocument(); @@ -86,7 +83,7 @@ test('rendering and switching tabs for new role', async () => { expect(screen.getByLabelText('Description')).toHaveValue(''); expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); expect(fromFauxYaml(await getTextEditorContents())).toEqual( withDefaults({ kind: 'role', @@ -103,7 +100,7 @@ test('rendering and switching tabs for new role', async () => { ); expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); - await user.click(screen.getByRole('tab', { name: 'Standard' })); + await user.click(getStandardEditorTab()); await screen.findByLabelText('Role Name'); expect( screen.queryByRole('button', { name: /Reset to Standard Settings/i }) @@ -127,14 +124,11 @@ test('rendering and switching tabs for a non-standard role', async () => { originalRole={{ object: originalRole, yaml: originalYaml }} /> ); - expect(screen.getByRole('tab', { name: 'YAML' })).toHaveAttribute( - 'aria-selected', - 'true' - ); + expect(getYamlEditorTab()).toHaveAttribute('aria-selected', 'true'); expect(fromFauxYaml(await getTextEditorContents())).toEqual(originalRole); expect(screen.getByRole('button', { name: 'Update Role' })).toBeDisabled(); - await user.click(screen.getByRole('tab', { name: 'Standard' })); + await user.click(getStandardEditorTab()); expect( screen.getByRole('button', { name: 'Reset to Standard Settings' }) ).toBeVisible(); @@ -142,12 +136,12 @@ test('rendering and switching tabs for a non-standard role', async () => { expect(screen.getByLabelText('Description')).toHaveValue(''); expect(screen.getByRole('button', { name: 'Update Role' })).toBeDisabled(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); expect(fromFauxYaml(await getTextEditorContents())).toEqual(originalRole); expect(screen.getByRole('button', { name: 'Update Role' })).toBeDisabled(); // Switch once again, reset to standard - await user.click(screen.getByRole('tab', { name: 'Standard' })); + await user.click(getStandardEditorTab()); expect(screen.getByRole('button', { name: 'Update Role' })).toBeDisabled(); await user.click( screen.getByRole('button', { name: 'Reset to Standard Settings' }) @@ -155,13 +149,35 @@ test('rendering and switching tabs for a non-standard role', async () => { expect(screen.getByRole('button', { name: 'Update Role' })).toBeEnabled(); await user.type(screen.getByLabelText('Description'), 'some description'); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); const editorContents = fromFauxYaml(await getTextEditorContents()); expect(editorContents.metadata.description).toBe('some description'); expect(editorContents.spec.deny).toEqual({}); expect(screen.getByRole('button', { name: 'Update Role' })).toBeEnabled(); }); +test('no double conversions when clicking already active tabs', async () => { + render(); + await user.click(getYamlEditorTab()); + await user.click(getStandardEditorTab()); + await user.type(screen.getByLabelText('Role Name'), '_2'); + await user.click(getStandardEditorTab()); + expect(screen.getByLabelText('Role Name')).toHaveValue('new_role_name_2'); + + await user.click(getYamlEditorTab()); + await user.clear(await findTextEditor()); + await user.type( + await findTextEditor(), + // Note: this is actually correct JSON syntax; the testing library uses + // braces for special keys, so we need to use double opening braces. + '{{"kind":"role", metadata:{{"name":"new_role_name_3"}}' + ); + await user.click(getYamlEditorTab()); + expect(await getTextEditorContents()).toBe( + '{"kind":"role", metadata:{"name":"new_role_name_3"}}' + ); +}); + test('canceling standard editor', async () => { const onCancel = jest.fn(); render(); @@ -175,7 +191,7 @@ test('canceling standard editor', async () => { test('canceling yaml editor', async () => { const onCancel = jest.fn(); render(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); await user.click(screen.getByRole('button', { name: 'Cancel' })); expect(onCancel).toHaveBeenCalled(); expect(userEventService.captureUserEvent).toHaveBeenCalledWith({ @@ -222,7 +238,7 @@ test('saving a new role after editing as YAML', async () => { render(); expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); await user.clear(await findTextEditor()); await user.type(await findTextEditor(), '{{"foo":"bar"}'); await user.click(screen.getByRole('button', { name: 'Create Role' })); @@ -247,7 +263,7 @@ test('error while yamlifying', async () => { .spyOn(yamlService, 'stringify') .mockRejectedValue(new Error('me no speak yaml')); render(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); + await user.click(getYamlEditorTab()); expect(screen.getByText('me no speak yaml')).toBeVisible(); }); @@ -256,8 +272,8 @@ test('error while parsing', async () => { .spyOn(yamlService, 'parse') .mockRejectedValue(new Error('me no speak yaml')); render(); - await user.click(screen.getByRole('tab', { name: 'YAML' })); - await user.click(screen.getByRole('tab', { name: 'Standard' })); + await user.click(getYamlEditorTab()); + await user.click(getStandardEditorTab()); expect(screen.getByText('me no speak yaml')).toBeVisible(); }); @@ -280,6 +296,12 @@ const TestRoleEditor = (props: RoleEditorProps) => { ); }; +const getStandardEditorTab = () => + screen.getByRole('tab', { name: 'Switch to standard editor' }); + +const getYamlEditorTab = () => + screen.getByRole('tab', { name: 'Switch to YAML editor' }); + const findTextEditor = async () => within(await screen.findByTestId('text-editor-container')).getByRole( 'textbox' diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx index cadef43e0ce26..38e2902ac2878 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx @@ -16,8 +16,8 @@ * along with this program. If not, see . */ -import { Alert, Box, Flex } from 'design'; -import React, { useState } from 'react'; +import { Alert, Flex } from 'design'; +import React, { useId, useState } from 'react'; import { useAsync } from 'shared/hooks/useAsync'; import { Role, RoleWithYaml } from 'teleport/services/resources'; @@ -32,7 +32,7 @@ import { roleToRoleEditorModel as roleToRoleEditorModel, } from './standardmodel'; import { YamlEditorModel } from './yamlmodel'; -import { EditorTab, EditorTabs } from './EditorTabs'; +import { EditorTab } from './EditorTabs'; import { EditorHeader } from './EditorHeader'; import { StandardEditor } from './StandardEditor'; import { YamlEditor } from './YamlEditor'; @@ -59,6 +59,12 @@ export const RoleEditor = ({ onSave, onDelete, }: RoleEditorProps) => { + const idPrefix = useId(); + // These IDs are needed to connect accessibility attributes between the + // standard/YAML tab switcher and the switched panels. + const standardEditorId = `${idPrefix}-standard`; + const yamlEditorId = `${idPrefix}-yaml`; + const [standardModel, setStandardModel] = useState( () => { const role = originalRole?.object ?? newRole(); @@ -114,6 +120,10 @@ export const RoleEditor = ({ saveAttempt.status === 'processing'; async function onTabChange(activeIndex: EditorTab) { + // The code below is not idempotent, so we need to protect ourselves from + // an accidental model replacement. + if (activeIndex === selectedEditorTab) return; + switch (activeIndex) { case EditorTab.Standard: { if (!yamlModel.content) { @@ -160,7 +170,15 @@ export const RoleEditor = ({ return ( - + {saveAttempt.status === 'error' && ( {saveAttempt.statusText} @@ -176,32 +194,29 @@ export const RoleEditor = ({ {yamlifyAttempt.statusText} )} - - - {selectedEditorTab === EditorTab.Standard && ( - handleSave({ object })} - onCancel={handleCancel} - standardEditorModel={standardModel} - isProcessing={isProcessing} - onChange={setStandardModel} - /> +
+ handleSave({ object })} + onCancel={handleCancel} + standardEditorModel={standardModel} + isProcessing={isProcessing} + onChange={setStandardModel} + /> +
)} {selectedEditorTab === EditorTab.Yaml && ( - void (await handleSave({ yaml }))} - isProcessing={isProcessing} - onCancel={handleCancel} - originalRole={originalRole} - /> + + void (await handleSave({ yaml }))} + isProcessing={isProcessing} + onCancel={handleCancel} + originalRole={originalRole} + /> + )} ); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx index 4497cbea441d7..59a7af9928081 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx @@ -19,8 +19,8 @@ import { render, screen, userEvent } from 'design/utils/testing'; import React, { useState } from 'react'; -import { within } from '@testing-library/react'; -import Validation from 'shared/components/Validation'; +import { act, within } from '@testing-library/react'; +import Validation, { Validator } from 'shared/components/Validation'; import selectEvent from 'react-select-event'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; @@ -48,6 +48,7 @@ import { StandardEditorProps, WindowsDesktopAccessSpecSection, } from './StandardEditor'; +import { validateAccessSpec } from './validation'; const TestStandardEditor = (props: Partial) => { const ctx = createTeleportContext(); @@ -72,6 +73,8 @@ test('adding and removing sections', async () => { const user = userEvent.setup(); render(); expect(getAllSectionNames()).toEqual(['Role Metadata']); + await user.click(screen.getByRole('tab', { name: 'Resources' })); + expect(getAllSectionNames()).toEqual([]); await user.click( screen.getByRole('button', { name: 'Add New Specifications' }) @@ -85,7 +88,7 @@ test('adding and removing sections', async () => { ]); await user.click(screen.getByRole('menuitem', { name: 'Servers' })); - expect(getAllSectionNames()).toEqual(['Role Metadata', 'Servers']); + expect(getAllSectionNames()).toEqual(['Servers']); await user.click( screen.getByRole('button', { name: 'Add New Specifications' }) @@ -98,25 +101,21 @@ test('adding and removing sections', async () => { ]); await user.click(screen.getByRole('menuitem', { name: 'Kubernetes' })); - expect(getAllSectionNames()).toEqual([ - 'Role Metadata', - 'Servers', - 'Kubernetes', - ]); + expect(getAllSectionNames()).toEqual(['Servers', 'Kubernetes']); await user.click( within(getSectionByName('Servers')).getByRole('button', { name: 'Remove section', }) ); - expect(getAllSectionNames()).toEqual(['Role Metadata', 'Kubernetes']); + expect(getAllSectionNames()).toEqual(['Kubernetes']); await user.click( within(getSectionByName('Kubernetes')).getByRole('button', { name: 'Remove section', }) ); - expect(getAllSectionNames()).toEqual(['Role Metadata']); + expect(getAllSectionNames()).toEqual([]); }); test('collapsed sections still apply validation', async () => { @@ -137,6 +136,24 @@ test('collapsed sections still apply validation', async () => { expect(onSave).toHaveBeenCalled(); }); +test('invisible tabs still apply validation', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + render(); + // Intentionally cause a validation error. + await user.clear(screen.getByLabelText('Role Name')); + // Switch to a different tab. + await user.click(screen.getByRole('tab', { name: 'Resources' })); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + expect(onSave).not.toHaveBeenCalled(); + + // Switch back, make it valid. + await user.click(screen.getByRole('tab', { name: 'Invalid data Overview' })); + await user.type(screen.getByLabelText('Role Name'), 'foo'); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + expect(onSave).toHaveBeenCalled(); +}); + const getAllMenuItemNames = () => screen.queryAllByRole('menuitem').map(m => m.textContent); @@ -152,67 +169,105 @@ const StatefulSection = ({ defaultValue, component: Component, onChange, + validatorRef, }: { defaultValue: S; - component: React.ComponentType>; + component: React.ComponentType>; onChange(spec: S): void; + validatorRef?(v: Validator): void; }) => { const [model, setModel] = useState(defaultValue); + const validation = validateAccessSpec(model); return ( - { - setModel(spec); - onChange(spec); - }} - /> + {({ validator }) => { + validatorRef?.(validator); + return ( + { + setModel(spec); + onChange(spec); + }} + /> + ); + }} ); }; -test('ServerAccessSpecSection', async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - render( - - component={ServerAccessSpecSection} - defaultValue={newAccessSpec('node')} - onChange={onChange} - /> - ); - await user.click(screen.getByRole('button', { name: 'Add a Label' })); - await user.type(screen.getByPlaceholderText('label key'), 'some-key'); - await user.type(screen.getByPlaceholderText('label value'), 'some-value'); - await selectEvent.create(screen.getByLabelText('Logins'), 'root', { - createOptionText: 'Login: root', - }); - await selectEvent.create(screen.getByLabelText('Logins'), 'some-user', { - createOptionText: 'Login: some-user', +describe('ServerAccessSpecSection', () => { + const setup = () => { + const onChange = jest.fn(); + let validator: Validator; + render( + + component={ServerAccessSpecSection} + defaultValue={newAccessSpec('node')} + onChange={onChange} + validatorRef={v => { + validator = v; + }} + /> + ); + return { user: userEvent.setup(), onChange, validator }; + }; + + test('editing', async () => { + const { user, onChange } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + await user.type(screen.getByPlaceholderText('label key'), 'some-key'); + await user.type(screen.getByPlaceholderText('label value'), 'some-value'); + await selectEvent.create(screen.getByLabelText('Logins'), 'root', { + createOptionText: 'Login: root', + }); + await selectEvent.create(screen.getByLabelText('Logins'), 'some-user', { + createOptionText: 'Login: some-user', + }); + + expect(onChange).toHaveBeenLastCalledWith({ + kind: 'node', + labels: [{ name: 'some-key', value: 'some-value' }], + logins: [ + expect.objectContaining({ label: 'root', value: 'root' }), + expect.objectContaining({ label: 'some-user', value: 'some-user' }), + ], + } as ServerAccessSpec); }); - expect(onChange).toHaveBeenLastCalledWith({ - kind: 'node', - labels: [{ name: 'some-key', value: 'some-value' }], - logins: [ - expect.objectContaining({ label: 'root', value: 'root' }), - expect.objectContaining({ label: 'some-user', value: 'some-user' }), - ], - } as ServerAccessSpec); + test('validation', async () => { + const { user, validator } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + await selectEvent.create(screen.getByLabelText('Logins'), '*', { + createOptionText: 'Login: *', + }); + act(() => validator.validate()); + expect( + screen.getByPlaceholderText('label key') + ).toHaveAccessibleDescription('required'); + expect( + screen.getByText('Wildcard is not allowed in logins') + ).toBeInTheDocument(); + }); }); describe('KubernetesAccessSpecSection', () => { const setup = () => { const onChange = jest.fn(); + let validator: Validator; render( component={KubernetesAccessSpecSection} defaultValue={newAccessSpec('kube_cluster')} onChange={onChange} + validatorRef={v => { + validator = v; + }} /> ); - return { user: userEvent.setup(), onChange }; + return { user: userEvent.setup(), onChange, validator }; }; test('editing the spec', async () => { @@ -319,105 +374,199 @@ describe('KubernetesAccessSpecSection', () => { expect.objectContaining({ resources: [] }) ); }); + + test('validation', async () => { + const { user, validator } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + await user.click(screen.getByRole('button', { name: 'Add a Resource' })); + await user.clear(screen.getByLabelText('Name')); + await user.clear(screen.getByLabelText('Namespace')); + act(() => validator.validate()); + expect( + screen.getByPlaceholderText('label key') + ).toHaveAccessibleDescription('required'); + expect(screen.getByLabelText('Name')).toHaveAccessibleDescription( + 'Resource name is required, use "*" for any resource' + ); + expect(screen.getByLabelText('Namespace')).toHaveAccessibleDescription( + 'Namespace is required for resources of this kind' + ); + }); }); -test('AppAccessSpecSection', async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - render( - - component={AppAccessSpecSection} - defaultValue={newAccessSpec('app')} - onChange={onChange} - /> - ); +describe('AppAccessSpecSection', () => { + const setup = () => { + const onChange = jest.fn(); + let validator: Validator; + render( + + component={AppAccessSpecSection} + defaultValue={newAccessSpec('app')} + onChange={onChange} + validatorRef={v => { + validator = v; + }} + /> + ); + return { user: userEvent.setup(), onChange, validator }; + }; - 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( + const awsRoleArn = () => within(screen.getByRole('group', { name: 'AWS Role ARNs' })).getByRole( 'textbox' - ), - 'arn:aws:iam::123456789012:role/admin' - ); - await user.type( + ); + const azureIdentity = () => within(screen.getByRole('group', { name: 'Azure Identities' })).getByRole( 'textbox' - ), - '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin' - ); - await user.type( + ); + const gcpServiceAccount = () => within( screen.getByRole('group', { name: 'GCP Service Accounts' }) - ).getByRole('textbox'), - 'admin@some-project.iam.gserviceaccount.com' - ); - expect(onChange).toHaveBeenLastCalledWith({ - kind: 'app', - labels: [{ name: 'env', value: 'prod' }], - awsRoleARNs: ['arn:aws:iam::123456789012:role/admin'], - azureIdentities: [ - '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin', - ], - gcpServiceAccounts: ['admin@some-project.iam.gserviceaccount.com'], - } as AppAccessSpec); -}); + ).getByRole('textbox'); -test('DatabaseAccessSpecSection', async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - render( - - component={DatabaseAccessSpecSection} - defaultValue={newAccessSpec('db')} - onChange={onChange} - /> - ); + 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.type( + azureIdentity(), + '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin' + ); + await user.type( + gcpServiceAccount(), + 'admin@some-project.iam.gserviceaccount.com' + ); + expect(onChange).toHaveBeenLastCalledWith({ + kind: 'app', + labels: [{ name: 'env', value: 'prod' }], + awsRoleARNs: ['arn:aws:iam::123456789012:role/admin'], + azureIdentities: [ + '/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin', + ], + gcpServiceAccounts: ['admin@some-project.iam.gserviceaccount.com'], + } as AppAccessSpec); + }); - 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 selectEvent.create(screen.getByLabelText('Database Names'), 'stuff', { - createOptionText: 'Database Name: stuff', + 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(), '*'); + act(() => validator.validate()); + expect( + screen.getByPlaceholderText('label key') + ).toHaveAccessibleDescription('required'); + expect(awsRoleArn()).toHaveAccessibleDescription( + 'Wildcard is not allowed in AWS role ARNs' + ); + expect(azureIdentity()).toHaveAccessibleDescription( + 'Wildcard is not allowed in Azure identities' + ); + expect(gcpServiceAccount()).toHaveAccessibleDescription( + 'Wildcard is not allowed in GCP service accounts' + ); }); - await selectEvent.create(screen.getByLabelText('Database Users'), 'mary', { - createOptionText: 'Database User: mary', +}); + +describe('DatabaseAccessSpecSection', () => { + const setup = () => { + const onChange = jest.fn(); + let validator: Validator; + render( + + component={DatabaseAccessSpecSection} + defaultValue={newAccessSpec('db')} + onChange={onChange} + validatorRef={v => { + validator = v; + }} + /> + ); + return { user: userEvent.setup(), onChange, validator }; + }; + + 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 selectEvent.create(screen.getByLabelText('Database Names'), 'stuff', { + createOptionText: 'Database Name: stuff', + }); + await selectEvent.create(screen.getByLabelText('Database Users'), 'mary', { + createOptionText: 'Database User: mary', + }); + await selectEvent.create(screen.getByLabelText('Database Roles'), 'admin', { + createOptionText: 'Database Role: admin', + }); + 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' })], + } as DatabaseAccessSpec); }); - await selectEvent.create(screen.getByLabelText('Database Roles'), 'admin', { - createOptionText: 'Database Role: admin', + + test('validation', async () => { + const { user, validator } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + await selectEvent.create(screen.getByLabelText('Database Roles'), '*', { + createOptionText: 'Database Role: *', + }); + act(() => validator.validate()); + expect( + screen.getByPlaceholderText('label key') + ).toHaveAccessibleDescription('required'); + expect( + screen.getByText('Wildcard is not allowed in database roles') + ).toBeInTheDocument(); }); - 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' })], - } as DatabaseAccessSpec); }); -test('WindowsDesktopAccessSpecSection', async () => { - const user = userEvent.setup(); - const onChange = jest.fn(); - render( - - component={WindowsDesktopAccessSpecSection} - defaultValue={newAccessSpec('windows_desktop')} - onChange={onChange} - /> - ); +describe('WindowsDesktopAccessSpecSection', () => { + const setup = () => { + const onChange = jest.fn(); + let validator: Validator; + render( + + component={WindowsDesktopAccessSpecSection} + defaultValue={newAccessSpec('windows_desktop')} + onChange={onChange} + validatorRef={v => { + validator = v; + }} + /> + ); + return { user: userEvent.setup(), onChange, validator }; + }; - await user.click(screen.getByRole('button', { name: 'Add a Label' })); - await user.type(screen.getByPlaceholderText('label key'), 'os'); - await user.type(screen.getByPlaceholderText('label value'), 'win-xp'); - await selectEvent.create(screen.getByLabelText('Logins'), 'julio', { - createOptionText: 'Login: julio', + test('editing', async () => { + const { user, onChange } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + await user.type(screen.getByPlaceholderText('label key'), 'os'); + await user.type(screen.getByPlaceholderText('label value'), 'win-xp'); + await selectEvent.create(screen.getByLabelText('Logins'), 'julio', { + createOptionText: 'Login: julio', + }); + expect(onChange).toHaveBeenLastCalledWith({ + kind: 'windows_desktop', + labels: [{ name: 'os', value: 'win-xp' }], + logins: [expect.objectContaining({ label: 'julio', value: 'julio' })], + } as WindowsDesktopAccessSpec); + }); + + test('validation', async () => { + const { user, validator } = setup(); + await user.click(screen.getByRole('button', { name: 'Add a Label' })); + act(() => validator.validate()); + expect( + screen.getByPlaceholderText('label key') + ).toHaveAccessibleDescription('required'); }); - expect(onChange).toHaveBeenLastCalledWith({ - kind: 'windows_desktop', - labels: [{ name: 'os', value: 'win-xp' }], - logins: [expect.objectContaining({ label: 'julio', value: 'julio' })], - } as WindowsDesktopAccessSpec); }); const reactSelectValueContainer = (input: HTMLInputElement) => diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx index 9eefd10718705..bf1567ee235cd 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import React, { useState } from 'react'; +import React, { useId, useState } from 'react'; import { Box, ButtonIcon, @@ -28,21 +28,25 @@ import { Text, } from 'design'; import FieldInput from 'shared/components/FieldInput'; -import Validation, { Validator } from 'shared/components/Validation'; -import { requiredField } from 'shared/components/Validation/rules'; +import Validation, { + useValidation, + Validator, +} from 'shared/components/Validation'; +import { + precomputed, + ValidationResult, +} from 'shared/components/Validation/rules'; import * as Icon from 'design/Icon'; import { HoverTooltip, IconTooltip } from 'design/Tooltip'; import styled, { useTheme } from 'styled-components'; - import { MenuButton, MenuItem } from 'shared/components/MenuAction'; - import { FieldSelect, FieldSelectCreatable, } from 'shared/components/FieldSelect'; +import { SlideTabs } from 'design/SlideTabs'; import { Role, RoleWithYaml } from 'teleport/services/resources'; - import { LabelsInput } from 'teleport/components/LabelsInput'; import { FieldMultiInput } from '../../../../shared/components/FieldMultiInput/FieldMultiInput'; @@ -66,6 +70,17 @@ import { DatabaseAccessSpec, WindowsDesktopAccessSpec, } from './standardmodel'; +import { + validateRoleEditorModel, + MetadataValidationResult, + AccessSpecValidationResult, + ServerSpecValidationResult, + KubernetesSpecValidationResult, + KubernetesResourceValidationResult, + AppSpecValidationResult, + DatabaseSpecValidationResult, + WindowsDesktopSpecValidationResult, +} from './validation'; import { EditorSaveCancelButton } from './Shared'; import { RequiresResetToStandard } from './RequiresResetToStandard'; @@ -92,12 +107,27 @@ export const StandardEditor = ({ }: StandardEditorProps) => { const isEditing = !!originalRole; const { roleModel } = standardEditorModel; + const validation = validateRoleEditorModel(roleModel); /** All spec kinds except those that are already in the role. */ const allowedSpecKinds = allAccessSpecKinds.filter(k => roleModel.accessSpecs.every(as => as.kind !== k) ); + enum StandardEditorTab { + Overview, + Resources, + AdminRules, + Options, + } + + const [currentTab, setCurrentTab] = useState(StandardEditorTab.Overview); + const idPrefix = useId(); + const overviewTabId = `${idPrefix}-overview`; + const resourcesTabId = `${idPrefix}-resources`; + const adminRulesTabId = `${idPrefix}-admin-rules`; + const optionsTabId = `${idPrefix}-options`; + function handleSave(validator: Validator) { if (!validator.validate()) { return; @@ -169,53 +199,113 @@ export const StandardEditor = ({ mute={standardEditorModel.roleModel.requiresReset} data-testid="standard-editor" > - + + !s.valid) + ? validationErrorTabStatus + : undefined, + }, + { + key: StandardEditorTab.AdminRules, + title: 'Admin Rules', + controls: adminRulesTabId, + }, + { + key: StandardEditorTab.Options, + title: 'Options', + controls: optionsTabId, + }, + ]} + activeIndex={currentTab} + onChange={setCurrentTab} + /> + +
handleChange({ ...roleModel, metadata })} /> - {roleModel.accessSpecs.map(spec => ( - setAccessSpec(value)} - onRemove={() => removeAccessSpec(spec.kind)} - /> - ))} - - - - Add New Specifications - - } - buttonProps={{ - size: 'medium', - fill: 'filled', - disabled: isProcessing || allowedSpecKinds.length === 0, - }} - > - {allowedSpecKinds.map(kind => ( - addAccessSpec(kind)}> - {specSections[kind].title} - - ))} - - - +
+
+ + {roleModel.accessSpecs.map((spec, i) => { + const validationResult = validation.accessSpecs[i]; + return ( + setAccessSpec(value)} + onRemove={() => removeAccessSpec(spec.kind)} + /> + ); + })} + + + + Add New Specifications + + } + buttonProps={{ + size: 'medium', + fill: 'filled', + disabled: isProcessing || allowedSpecKinds.length === 0, + }} + > + {allowedSpecKinds.map(kind => ( + addAccessSpec(kind)}> + {specSections[kind].title} + + ))} + + + +
handleSave(validator)} @@ -233,28 +323,36 @@ export const StandardEditor = ({ ); }; -export type SectionProps = { +export type SectionProps = { value: T; isProcessing: boolean; + validation?: V; onChange?(value: T): void; }; +const validationErrorTabStatus = { + kind: 'danger', + ariaLabel: 'Invalid data', +} as const; + const MetadataSection = ({ value, isProcessing, + validation, onChange, -}: SectionProps) => ( +}: SectionProps) => (
onChange({ ...value, name: e.target.value })} /> ) => { const theme = useTheme(); const [expanded, setExpanded] = useState(true); const ExpandIcon = expanded ? Icon.Minus : Icon.Plus; const expandTooltip = expanded ? 'Collapse' : 'Expand'; + const validator = useValidation(); const handleExpand = (e: React.MouseEvent) => { // Don't let handle the event, we'll do it ourselves to keep @@ -311,7 +412,11 @@ const Section = ({ as="details" open={expanded} border={1} - borderColor={theme.colors.interactive.tonal.neutral[0]} + borderColor={ + validator.state.validating && !validation.valid + ? theme.colors.interactive.solid.danger.default + : theme.colors.interactive.tonal.neutral[0] + } borderRadius={3} > >; + component: React.ComponentType>; } > = { kube_cluster: { @@ -409,12 +514,16 @@ const specSections: Record< * A generic access spec section. Details are rendered by components from the * `specSections` map. */ -const AccessSpecSection = ({ +const AccessSpecSection = < + T extends AccessSpec, + V extends AccessSpecValidationResult, +>({ value, isProcessing, + validation, onChange, onRemove, -}: SectionProps & { +}: SectionProps & { onRemove?(): void; }) => { const { component: Body, title, tooltip } = specSections[value.kind]; @@ -425,8 +534,14 @@ const AccessSpecSection = ({ onRemove={onRemove} tooltip={tooltip} isProcessing={isProcessing} + validation={validation} > - +
); }; @@ -434,8 +549,9 @@ const AccessSpecSection = ({ export function ServerAccessSpecSection({ value, isProcessing, + validation, onChange, -}: SectionProps) { +}: SectionProps) { return ( <> @@ -445,6 +561,7 @@ export function ServerAccessSpecSection({ disableBtns={isProcessing} labels={value.labels} setLabels={labels => onChange?.({ ...value, labels })} + rule={precomputed(validation.fields.labels)} /> onChange?.({ ...value, logins })} + rule={precomputed(validation.fields.logins)} mt={3} mb={0} /> @@ -467,8 +585,9 @@ export function ServerAccessSpecSection({ export function KubernetesAccessSpecSection({ value, isProcessing, + validation, onChange, -}: SectionProps) { +}: SectionProps) { return ( <> onChange?.({ ...value, labels })} /> @@ -498,6 +618,7 @@ export function KubernetesAccessSpecSection({ onChange?.({ @@ -540,11 +661,13 @@ export function KubernetesAccessSpecSection({ function KubernetesResourceView({ value, + validation, isProcessing, onChange, onRemove, }: { value: KubernetesResourceModel; + validation: KubernetesResourceValidationResult; isProcessing: boolean; onChange(m: KubernetesResourceModel): void; onRemove(): void; @@ -590,6 +713,7 @@ function KubernetesResourceView({ } disabled={isProcessing} value={name} + rule={precomputed(validation.name)} onChange={e => onChange?.({ ...value, name: e.target.value })} /> onChange?.({ ...value, namespace: e.target.value })} /> ) { +}: SectionProps) { return ( @@ -632,6 +758,7 @@ export function AppAccessSpecSection({ disableBtns={isProcessing} labels={value.labels} setLabels={labels => onChange?.({ ...value, labels })} + rule={precomputed(validation.fields.labels)} /> onChange?.({ ...value, awsRoleARNs: arns })} + rule={precomputed(validation.fields.awsRoleARNs)} /> onChange?.({ ...value, azureIdentities: ids })} + rule={precomputed(validation.fields.azureIdentities)} /> onChange?.({ ...value, gcpServiceAccounts: accts })} + rule={precomputed(validation.fields.gcpServiceAccounts)} /> ); @@ -659,8 +789,9 @@ export function AppAccessSpecSection({ export function DatabaseAccessSpecSection({ value, isProcessing, + validation, onChange, -}: SectionProps) { +}: SectionProps) { return ( <> @@ -671,6 +802,7 @@ export function DatabaseAccessSpecSection({ disableBtns={isProcessing} labels={value.labels} setLabels={labels => onChange?.({ ...value, labels })} + rule={precomputed(validation.fields.labels)} /> onChange?.({ ...value, roles })} + rule={precomputed(validation.fields.roles)} mb={0} /> @@ -730,8 +863,9 @@ export function DatabaseAccessSpecSection({ export function WindowsDesktopAccessSpecSection({ value, isProcessing, + validation, onChange, -}: SectionProps) { +}: SectionProps) { return ( <> @@ -742,6 +876,7 @@ export function WindowsDesktopAccessSpecSection({ disableBtns={isProcessing} labels={value.labels} setLabels={labels => onChange?.({ ...value, labels })} + rule={precomputed(validation.fields.labels)} /> ; -type KubernetesResourceKind = - | '*' - | 'pod' - | 'secret' - | 'configmap' - | 'namespace' - | 'service' - | 'serviceaccount' - | 'kube_node' - | 'persistentvolume' - | 'persistentvolumeclaim' - | 'deployment' - | 'replicaset' - | 'statefulset' - | 'daemonset' - | 'clusterrole' - | 'kube_role' - | 'clusterrolebinding' - | 'rolebinding' - | 'cronjob' - | 'job' - | 'certificatesigningrequest' - | 'ingress'; /** * All possible resource kind drop-down options. This array needs to be kept in @@ -172,19 +151,6 @@ export const kubernetesResourceKindOptions: KubernetesResourceKindOption[] = [ ]; type KubernetesVerbOption = Option; -type KubernetesVerb = - | '*' - | 'get' - | 'create' - | 'update' - | 'patch' - | 'delete' - | 'list' - | 'watch' - | 'deletecollection' - | 'exec' - | 'portforward'; - /** * All possible Kubernetes verb drop-down options. This array needs to be kept * in sync with `KubernetesVerbs` in `api/types/constants.go. @@ -590,7 +556,7 @@ export function labelsModelToLabels(uiLabels: UILabel[]): Labels { return labels; } -function optionsToStrings(opts: readonly Option[]): string[] { +function optionsToStrings(opts: readonly Option[]): T[] { return opts.map(opt => opt.value); } diff --git a/web/packages/teleport/src/Roles/RoleEditor/validation.ts b/web/packages/teleport/src/Roles/RoleEditor/validation.ts new file mode 100644 index 0000000000000..95cde89ed036c --- /dev/null +++ b/web/packages/teleport/src/Roles/RoleEditor/validation.ts @@ -0,0 +1,168 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { + arrayOf, + requiredField, + RuleSetValidationResult, + runRules, + ValidationResult, +} from 'shared/components/Validation/rules'; + +import { Option } from 'shared/components/Select'; + +import { KubernetesResourceKind } from 'teleport/services/resources'; + +import { nonEmptyLabels } from 'teleport/components/LabelsInput/LabelsInput'; + +import { + AccessSpec, + KubernetesResourceModel, + MetadataModel, + RoleEditorModel, +} from './standardmodel'; + +const kubernetesClusterWideResourceKinds: KubernetesResourceKind[] = [ + 'namespace', + 'kube_node', + 'persistentvolume', + 'clusterrole', + 'clusterrolebinding', + 'certificatesigningrequest', +]; + +export function validateRoleEditorModel({ + metadata, + accessSpecs, +}: RoleEditorModel) { + return { + metadata: validateMetadata(metadata), + accessSpecs: accessSpecs.map(validateAccessSpec), + }; +} + +function validateMetadata(model: MetadataModel): MetadataValidationResult { + return runRules(model, metadataRules); +} + +const metadataRules = { name: requiredField('Role name is required') }; +export type MetadataValidationResult = RuleSetValidationResult< + typeof metadataRules +>; + +export function validateAccessSpec( + spec: AccessSpec +): AccessSpecValidationResult { + const { kind } = spec; + switch (kind) { + case 'kube_cluster': + return runRules(spec, kubernetesValidationRules); + case 'node': + return runRules(spec, serverValidationRules); + case 'app': + return runRules(spec, appSpecValidationRules); + case 'db': + return runRules(spec, databaseSpecValidationRules); + case 'windows_desktop': + return runRules(spec, windowsDesktopSpecValidationRules); + default: + kind satisfies never; + } +} + +export type AccessSpecValidationResult = + | ServerSpecValidationResult + | KubernetesSpecValidationResult + | AppSpecValidationResult + | DatabaseSpecValidationResult + | WindowsDesktopSpecValidationResult; + +const validKubernetesResource = (res: KubernetesResourceModel) => () => { + const name = requiredField( + 'Resource name is required, use "*" for any resource' + )(res.name)(); + const namespace = kubernetesClusterWideResourceKinds.includes(res.kind.value) + ? { valid: true } + : requiredField('Namespace is required for resources of this kind')( + res.namespace + )(); + return { + valid: name.valid && namespace.valid, + name, + namespace, + }; +}; +export type KubernetesResourceValidationResult = { + name: ValidationResult; + namespace: ValidationResult; +}; + +const kubernetesValidationRules = { + labels: nonEmptyLabels, + resources: arrayOf(validKubernetesResource), +}; +export type KubernetesSpecValidationResult = RuleSetValidationResult< + typeof kubernetesValidationRules +>; + +const noWildcard = (message: string) => (value: string) => () => { + const valid = value !== '*'; + return { valid, message: valid ? '' : message }; +}; + +const noWildcardOptions = (message: string) => (options: Option[]) => () => { + const valid = options.every(o => o.value !== '*'); + return { valid, message: valid ? '' : message }; +}; + +const serverValidationRules = { + labels: nonEmptyLabels, + logins: noWildcardOptions('Wildcard is not allowed in logins'), +}; +export type ServerSpecValidationResult = RuleSetValidationResult< + typeof serverValidationRules +>; + +const appSpecValidationRules = { + labels: nonEmptyLabels, + awsRoleARNs: arrayOf(noWildcard('Wildcard is not allowed in AWS role ARNs')), + azureIdentities: arrayOf( + noWildcard('Wildcard is not allowed in Azure identities') + ), + gcpServiceAccounts: arrayOf( + noWildcard('Wildcard is not allowed in GCP service accounts') + ), +}; +export type AppSpecValidationResult = RuleSetValidationResult< + typeof appSpecValidationRules +>; + +const databaseSpecValidationRules = { + labels: nonEmptyLabels, + roles: noWildcardOptions('Wildcard is not allowed in database roles'), +}; +export type DatabaseSpecValidationResult = RuleSetValidationResult< + typeof databaseSpecValidationRules +>; + +const windowsDesktopSpecValidationRules = { + labels: nonEmptyLabels, +}; +export type WindowsDesktopSpecValidationResult = RuleSetValidationResult< + typeof windowsDesktopSpecValidationRules +>; diff --git a/web/packages/teleport/src/Roles/Roles.tsx b/web/packages/teleport/src/Roles/Roles.tsx index 3bf968247b6ba..2419052bf8db1 100644 --- a/web/packages/teleport/src/Roles/Roles.tsx +++ b/web/packages/teleport/src/Roles/Roles.tsx @@ -184,7 +184,7 @@ export function Roles(props: State) { )} - + {convertAttempt.status === 'processing' && ( ; export type KubernetesResource = { - kind?: string; + kind?: KubernetesResourceKind; name?: string; namespace?: string; - verbs?: string[]; + verbs?: KubernetesVerb[]; }; +/** + * Supported Kubernetes resource kinds. This type needs to be kept in sync with + * `KubernetesResourcesKinds` in `api/types/constants.go, as well as + * `kubernetesResourceKindOptions` in + * `web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts`. + */ +export type KubernetesResourceKind = + | '*' + | 'pod' + | 'secret' + | 'configmap' + | 'namespace' + | 'service' + | 'serviceaccount' + | 'kube_node' + | 'persistentvolume' + | 'persistentvolumeclaim' + | 'deployment' + | 'replicaset' + | 'statefulset' + | 'daemonset' + | 'clusterrole' + | 'kube_role' + | 'clusterrolebinding' + | 'rolebinding' + | 'cronjob' + | 'job' + | 'certificatesigningrequest' + | 'ingress'; + +/** + * Supported Kubernetes resource verbs. This type needs to be kept in sync with + * `KubernetesVerbs` in `api/types/constants.go, as well as + * `kubernetesVerbOptions` in + * `web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts`. + */ +export type KubernetesVerb = + | '*' + | 'get' + | 'create' + | 'update' + | 'patch' + | 'delete' + | 'list' + | 'watch' + | 'deletecollection' + | 'exec' + | 'portforward'; + /** * Teleport role options in full format, as returned from Teleport API. Note * that its fields follow the snake case convention to match the wire format.