diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx index 596edbb06c332..32a2440f800dc 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx @@ -31,11 +31,10 @@ import { EditorHeader } from './EditorHeader'; import { EditorTab } from './EditorTabs'; import { StandardEditor } from './StandardEditor/StandardEditor'; import { - newRole, roleEditorModelToRole, roleToRoleEditorModel, - StandardEditorModel, } from './StandardEditor/standardmodel'; +import { useStandardModel } from './StandardEditor/useStandardModel'; import { YamlEditor } from './YamlEditor'; import { YamlEditorModel } from './yamlmodel'; @@ -65,16 +64,7 @@ export const RoleEditor = ({ const standardEditorId = `${idPrefix}-standard`; const yamlEditorId = `${idPrefix}-yaml`; - const [standardModel, setStandardModel] = useState( - () => { - const role = originalRole?.object ?? newRole(); - const roleModel = roleToRoleEditorModel(role, role); - return { - roleModel, - isDirty: !originalRole, // New role is dirty by default. - }; - } - ); + const [standardModel, dispatch] = useStandardModel(originalRole?.object); const [yamlModel, setYamlModel] = useState({ content: originalRole?.yaml ?? '', @@ -141,9 +131,9 @@ export const RoleEditor = ({ // Abort if there's an error. Don't switch the tab or set the model. if (err) return; - setStandardModel({ - roleModel, - isDirty: yamlModel.isDirty, + dispatch({ + type: 'set-role-model', + payload: roleModel, }); break; } @@ -213,7 +203,7 @@ export const RoleEditor = ({ onCancel={handleCancel} standardEditorModel={standardModel} isProcessing={isProcessing} - onChange={setStandardModel} + dispatch={dispatch} /> )} diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.test.tsx index b943938c68cd5..0eabc61fd90db 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.test.tsx @@ -26,29 +26,29 @@ import { ResourceKind } from 'teleport/services/resources'; import { AccessRules } from './AccessRules'; import { RuleModel } from './standardmodel'; -import { StatefulSection } from './StatefulSection'; -import { AccessRuleValidationResult, validateAccessRule } from './validation'; +import { StatefulSectionWithDispatch } from './StatefulSection'; +import { AccessRuleValidationResult } from './validation'; describe('AccessRules', () => { const setup = () => { - const onChange = jest.fn(); + const modelRef = jest.fn(); let validator: Validator; render( - + + selector={m => m.roleModel.rules} + validationSelector={m => m.validationResult.rules} component={AccessRules} - defaultValue={[]} - onChange={onChange} validatorRef={v => { validator = v; }} - validate={rules => rules.map(validateAccessRule)} + modelRef={modelRef} /> ); - return { user: userEvent.setup(), onChange, validator }; + return { user: userEvent.setup(), modelRef, validator }; }; test('editing', async () => { - const { user, onChange } = setup(); + const { user, modelRef } = setup(); await user.click(screen.getByRole('button', { name: 'Add New' })); await selectEvent.select(screen.getByLabelText('Resources'), [ 'db', @@ -58,7 +58,7 @@ describe('AccessRules', () => { 'list', 'read', ]); - expect(onChange).toHaveBeenLastCalledWith([ + expect(modelRef).toHaveBeenLastCalledWith([ { id: expect.any(String), resources: [ diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.tsx index eeacafce05d0d..11c5a02b58b1e 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/AccessRules.tsx @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +import { memo } from 'react'; import { components, MultiValueProps } from 'react-select'; import styled from 'styled-components'; @@ -29,9 +30,8 @@ import { } from 'shared/components/FieldSelect'; import { precomputed } from 'shared/components/Validation/rules'; -import { SectionBox, SectionProps } from './sections'; +import { SectionBox, SectionPropsWithDispatch } from './sections'; import { - newRuleModel, ResourceKindOption, resourceKindOptions, resourceKindOptionsMap, @@ -40,20 +40,18 @@ import { } from './standardmodel'; import { AccessRuleValidationResult } from './validation'; -export function AccessRules({ +/** + * Access rules tab. This component is memoized to optimize performance; make + * sure that the properties don't change unless necessary. + */ +export const AccessRules = memo(function AccessRules({ value, isProcessing, validation, - onChange, -}: SectionProps) { + dispatch, +}: SectionPropsWithDispatch) { function addRule() { - onChange?.([...value, newRuleModel()]); - } - function setRule(rule: RuleModel) { - onChange?.(value.map(r => (r.id === rule.id ? rule : r))); - } - function removeRule(id: string) { - onChange?.(value.filter(r => r.id !== id)); + dispatch({ type: 'add-access-rule' }); } return ( @@ -62,9 +60,8 @@ export function AccessRules({ key={rule.id} isProcessing={isProcessing} value={rule} - onChange={setRule} validation={validation[i]} - onRemove={() => removeRule(rule.id)} + dispatch={dispatch} /> ))} @@ -73,18 +70,21 @@ export function AccessRules({ ); -} +}); -function AccessRule({ +const AccessRule = memo(function AccessRule({ value, isProcessing, validation, - onChange, - onRemove, -}: SectionProps & { - onRemove?(): void; -}) { - const { resources, verbs } = value; + dispatch, +}: SectionPropsWithDispatch) { + const { id, resources, verbs } = value; + function setRule(rule: RuleModel) { + dispatch({ type: 'set-access-rule', payload: rule }); + } + function removeRule() { + dispatch({ type: 'remove-access-rule', payload: { id } }); + } return ( onChange?.({ ...value, resources: r })} + onChange={r => setRule?.({ ...value, resources: r })} rule={precomputed(validation.fields.resources)} /> onChange?.({ ...value, verbs: v })} + onChange={v => setRule?.({ ...value, verbs: v })} rule={precomputed(validation.fields.verbs)} mb={0} /> ); -} +}); const ResourceKindSelect = styled( FieldSelectCreatable diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Options.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Options.tsx index 0cd0fc61b1f16..cbde6cb188559 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Options.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Options.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { useId } from 'react'; +import { memo, useId } from 'react'; import styled, { useTheme } from 'styled-components'; import Box from 'design/Box'; @@ -35,7 +35,11 @@ import { sessionRecordingModeOptions, } from './standardmodel'; -export function Options({ +/** + * Options tab. This component is memoized to optimize performance; make sure + * that the properties don't change unless necessary. + */ +export const Options = memo(function Options({ value, isProcessing, onChange, @@ -189,7 +193,7 @@ export function Options({ /> ); -} +}); const OptionsGridContainer = styled(Box)` display: grid; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx index 84b077c090a16..5090d7ac6ba53 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx @@ -16,13 +16,14 @@ * along with this program. If not, see . */ +import { memo } from 'react'; import styled, { useTheme } from 'styled-components'; import Box from 'design/Box'; import { ButtonSecondary } from 'design/Button'; import ButtonIcon from 'design/ButtonIcon'; import Flex from 'design/Flex'; -import { Add, Trash } from 'design/Icon'; +import { Add, Plus, Trash } from 'design/Icon'; import { Mark } from 'design/Mark'; import Text, { H4 } from 'design/Text'; import FieldInput from 'shared/components/FieldInput'; @@ -30,11 +31,12 @@ import { FieldMultiInput } from 'shared/components/FieldMultiInput/FieldMultiInp import FieldSelect, { FieldSelectCreatable, } from 'shared/components/FieldSelect'; +import { MenuButton, MenuItem } from 'shared/components/MenuAction'; import { precomputed } from 'shared/components/Validation/rules'; import { LabelsInput } from 'teleport/components/LabelsInput'; -import { SectionBox, SectionProps } from './sections'; +import { SectionBox, SectionProps, SectionPropsWithDispatch } from './sections'; import { AppAccess, DatabaseAccess, @@ -58,6 +60,87 @@ import { WindowsDesktopAccessValidationResult, } from './validation'; +/** + * Resources tab. This component is memoized to optimize performance; make sure + * that the properties don't change unless necessary. + */ +export const ResourcesTab = memo(function ResourcesTab({ + value, + isProcessing, + validation, + dispatch, +}: SectionPropsWithDispatch< + ResourceAccess[], + ResourceAccessValidationResult[] +>) { + /** All resource access kinds except those that are already in the role. */ + const allowedResourceAccessKinds = allResourceAccessKinds.filter(k => + value.every(as => as.kind !== k) + ); + + const addResourceAccess = (kind: ResourceAccessKind) => + dispatch({ type: 'add-resource-access', payload: { kind } }); + + return ( + + {value.map((res, i) => { + return ( + + ); + })} + + + + Add New Resource Access + + } + buttonProps={{ + size: 'medium', + fill: 'filled', + disabled: isProcessing || allowedResourceAccessKinds.length === 0, + }} + > + {allowedResourceAccessKinds.map(kind => ( + addResourceAccess(kind)}> + {resourceAccessSections[kind].title} + + ))} + + + + ); +}); + +/** + * All resource access kinds, in order of appearance in the resource kind + * dropdown. + */ +const allResourceAccessKinds: ResourceAccessKind[] = [ + 'kube_cluster', + 'node', + 'app', + 'db', + 'windows_desktop', +]; + /** Maps resource access kind to UI component configuration. */ export const resourceAccessSections: Record< ResourceAccessKind, @@ -98,28 +181,34 @@ export const resourceAccessSections: Record< * A generic resource section. Details are rendered by components from the * `resourceAccessSections` map. */ -export const ResourceAccessSection = < +export const ResourceAccessSection = memo(function ResourceAccessSectionRaw< T extends ResourceAccess, V extends ResourceAccessValidationResult, >({ value, isProcessing, validation, - onChange, - onRemove, -}: SectionProps & { - onRemove?(): void; -}) => { + dispatch, +}: SectionPropsWithDispatch) { const { component: Body, title, tooltip, } = resourceAccessSections[value.kind]; + + function handleChange(val: T) { + dispatch({ type: 'set-resource-access', payload: val }); + } + + function handleRemove() { + dispatch({ type: 'remove-resource-access', payload: { kind: value.kind } }); + } + return ( ); -}; +}); export function ServerAccessSection({ value, diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx index 6732dd604225d..95ebd8266709c 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx @@ -17,27 +17,21 @@ */ import { within } from '@testing-library/react'; -import { useState } from 'react'; +import selectEvent from 'react-select-event'; import { render, screen, userEvent } from 'design/utils/testing'; import Validation from 'shared/components/Validation'; import { createTeleportContext } from 'teleport/mocks/contexts'; +import { Role } from 'teleport/services/resources'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; import { StandardEditor, StandardEditorProps } from './StandardEditor'; -import { - newRole, - roleToRoleEditorModel, - StandardEditorModel, -} from './standardmodel'; +import { useStandardModel } from './useStandardModel'; const TestStandardEditor = (props: Partial) => { const ctx = createTeleportContext(); - const [model, setModel] = useState({ - roleModel: roleToRoleEditorModel(newRole()), - isDirty: true, - }); + const [model, dispatch] = useStandardModel(); return ( @@ -45,7 +39,7 @@ const TestStandardEditor = (props: Partial) => { originalRole={null} standardEditorModel={model} isProcessing={false} - onChange={setModel} + dispatch={dispatch} {...props} /> @@ -138,6 +132,33 @@ test('invisible tabs still apply validation', async () => { expect(onSave).toHaveBeenCalled(); }); +test('edits metadata', async () => { + const user = userEvent.setup(); + let role: Role | undefined; + const onSave = (r: Role) => (role = r); + render(); + await user.type(screen.getByLabelText('Description'), 'foo'); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + expect(role.metadata.description).toBe('foo'); +}); + +test('edits resource access', async () => { + const user = userEvent.setup(); + let role: Role | undefined; + const onSave = (r: Role) => (role = r); + render(); + await user.click(screen.getByRole('tab', { name: 'Resources' })); + await user.click( + screen.getByRole('button', { name: 'Add New Resource Access' }) + ); + await user.click(screen.getByRole('menuitem', { name: 'Servers' })); + await selectEvent.create(screen.getByLabelText('Logins'), 'ec2-user', { + createOptionText: 'Login: ec2-user', + }); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + expect(role.spec.allow.logins).toEqual(['{{internal.logins}}', 'ec2-user']); +}); + const getAllMenuItemNames = () => screen.queryAllByRole('menuitem').map(m => m.textContent); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx index 86f9a86103cb4..bd2b808c5e665 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx @@ -16,13 +16,11 @@ * along with this program. If not, see . */ -import { useId, useState } from 'react'; +import { useCallback, useId, useState } from 'react'; import styled from 'styled-components'; import { Box, Flex } from 'design'; -import * as Icon from 'design/Icon'; import { SlideTabs } from 'design/SlideTabs'; -import { MenuButton, MenuItem } from 'shared/components/MenuAction'; import { useValidation } from 'shared/components/Validation'; import { Role, RoleWithYaml } from 'teleport/services/resources'; @@ -32,19 +30,13 @@ import { AccessRules } from './AccessRules'; import { MetadataSection } from './MetadataSection'; import { Options } from './Options'; import { RequiresResetToStandard } from './RequiresResetToStandard'; -import { ResourceAccessSection, resourceAccessSections } from './Resources'; +import { ResourcesTab } from './Resources'; import { - hasModifiedFields, - newResourceAccess, OptionsModel, - ResourceAccess, - ResourceAccessKind, - RoleEditorModel, roleEditorModelToRole, - RuleModel, StandardEditorModel, } from './standardmodel'; -import { validateRoleEditorModel } from './validation'; +import { StandardModelDispatcher } from './useStandardModel'; export type StandardEditorProps = { originalRole: RoleWithYaml; @@ -52,7 +44,7 @@ export type StandardEditorProps = { isProcessing?: boolean; onSave?(r: Role): void; onCancel?(): void; - onChange?(s: StandardEditorModel): void; + dispatch: StandardModelDispatcher; }; /** @@ -65,16 +57,10 @@ export const StandardEditor = ({ isProcessing, onSave, onCancel, - onChange, + dispatch, }: StandardEditorProps) => { const isEditing = !!originalRole; - const { roleModel } = standardEditorModel; - const validation = validateRoleEditorModel(roleModel); - - /** All resource access kinds except those that are already in the role. */ - const allowedResourceAccessKinds = allResourceAccessKinds.filter(k => - roleModel.resources.every(as => as.kind !== k) - ); + const { roleModel, validationResult } = standardEditorModel; enum StandardEditorTab { Overview, @@ -99,60 +85,11 @@ export const StandardEditor = ({ onSave?.(roleEditorModelToRole(standardEditorModel.roleModel)); } - function handleChange(modified: Partial) { - const updatedResourceModel: RoleEditorModel = { - ...roleModel, - ...modified, - }; - - onChange?.({ - ...standardEditorModel, - roleModel: updatedResourceModel, - isDirty: hasModifiedFields(updatedResourceModel, originalRole?.object), - }); - } - - function addResourceAccess(kind: ResourceAccessKind) { - handleChange({ - ...standardEditorModel.roleModel, - resources: [ - ...standardEditorModel.roleModel.resources, - newResourceAccess(kind), - ], - }); - } - - function removeResourceAccess(kind: ResourceAccessKind) { - handleChange({ - ...standardEditorModel.roleModel, - resources: standardEditorModel.roleModel.resources.filter( - s => s.kind !== kind - ), - }); - } - - function setResourceAccess(value: ResourceAccess) { - handleChange({ - ...standardEditorModel.roleModel, - resources: standardEditorModel.roleModel.resources.map(original => - original.kind === value.kind ? value : original - ), - }); - } - - function setRules(rules: RuleModel[]) { - handleChange({ - ...standardEditorModel.roleModel, - rules, - }); - } - - function setOptions(options: OptionsModel) { - handleChange({ - ...standardEditorModel, - options, - }); - } + const setOptions = useCallback( + (options: OptionsModel) => + dispatch({ type: 'set-options', payload: options }), + [dispatch] + ); return ( <> @@ -175,7 +112,7 @@ export const StandardEditor = ({ title: 'Overview', controls: overviewTabId, status: - validator.state.validating && !validation.metadata.valid + validator.state.validating && !validationResult.metadata.valid ? validationErrorTabStatus : undefined, }, @@ -185,7 +122,7 @@ export const StandardEditor = ({ controls: resourcesTabId, status: validator.state.validating && - validation.resources.some(s => !s.valid) + validationResult.resources.some(s => !s.valid) ? validationErrorTabStatus : undefined, }, @@ -195,7 +132,7 @@ export const StandardEditor = ({ controls: accessRulesTabId, status: validator.state.validating && - validation.rules.some(s => !s.valid) + validationResult.rules.some(s => !s.valid) ? validationErrorTabStatus : undefined, }, @@ -227,8 +164,10 @@ export const StandardEditor = ({ handleChange({ ...roleModel, metadata })} + validation={validationResult.metadata} + onChange={metadata => + dispatch({ type: 'set-metadata', payload: metadata }) + } /> - - {roleModel.resources.map((res, i) => { - const validationResult = validation.resources[i]; - return ( - setResourceAccess(value)} - onRemove={() => removeResourceAccess(res.kind)} - /> - ); - })} - - - - Add New Resource Access - - } - buttonProps={{ - size: 'medium', - fill: 'filled', - disabled: - isProcessing || allowedResourceAccessKinds.length === 0, - }} - > - {allowedResourceAccessKinds.map(kind => ( - addResourceAccess(kind)} - > - {resourceAccessSections[kind].title} - - ))} - - - + ` flex-direction: column; flex: 1; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx index 3be195cbfa696..387378909b8e0 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx @@ -16,11 +16,14 @@ * along with this program. If not, see . */ -import React, { useState } from 'react'; +import React, { useReducer } from 'react'; import Validation, { Validator } from 'shared/components/Validation'; -import { SectionProps } from './sections'; +import { SectionProps, SectionPropsWithDispatch } from './sections'; +import { StandardEditorModel } from './standardmodel'; +import { useStandardModel } from './useStandardModel'; +import { withDefaults } from './withDefaults'; /** A helper for testing editor section components. */ export function StatefulSection({ @@ -34,10 +37,25 @@ export function StatefulSection({ component: React.ComponentType>; onChange(model: Model): void; validatorRef?(v: Validator): void; - validate(arg: Model): ValidationResult; + validate( + model: Model, + previousModel: Model, + previousResult: ValidationResult + ): ValidationResult; }) { - const [model, setModel] = useState(defaultValue); - const validation = validate(model); + const [{ model, validationResult }, dispatch] = useReducer( + ( + { model, validationResult: previousValidationResult }, + newModel: Model + ) => ({ + model: newModel, + validationResult: validate(newModel, model, previousValidationResult), + }), + { + model: defaultValue, + validationResult: validate(defaultValue, undefined, undefined), + } + ); return ( {({ validator }) => { @@ -45,11 +63,11 @@ export function StatefulSection({ return ( { - setModel(model); - onChange(model); + onChange={newModel => { + dispatch(newModel); + onChange(newModel); }} /> ); @@ -57,3 +75,43 @@ export function StatefulSection({ ); } + +const minimalRole = withDefaults({ + metadata: { name: 'foobar' }, + version: 'v7', +}); + +/** A helper for testing editor section components. */ +export function StatefulSectionWithDispatch({ + selector, + validationSelector, + component: Component, + validatorRef, + modelRef, +}: { + selector(m: StandardEditorModel): Model; + validationSelector(m: StandardEditorModel): ValidationResult; + component: React.ComponentType>; + validatorRef?(v: Validator): void; + modelRef?(m: Model): void; +}) { + const [state, dispatch] = useStandardModel(minimalRole); + const model = selector(state); + const validation = validationSelector(state); + modelRef?.(model); + return ( + + {({ validator }) => { + validatorRef?.(validator); + return ( + + ); + }} + + ); +} diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx index d99d8dd5fde26..272affddc5132 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx @@ -28,6 +28,9 @@ import { HoverTooltip, IconTooltip } from 'design/Tooltip'; import { useValidation } from 'shared/components/Validation'; import { ValidationResult } from 'shared/components/Validation/rules'; +import { StandardModelDispatcher } from './useStandardModel'; + +/** Properties of a section that uses plain callbacks to change the model. */ export type SectionProps = { value: Model; isProcessing: boolean; @@ -35,6 +38,14 @@ export type SectionProps = { onChange?(value: Model): void; }; +/** Properties of a section that uses a dispatcher to change the model. */ +export type SectionPropsWithDispatch = { + value: Model; + isProcessing: boolean; + validation?: ValidationResult; + dispatch: StandardModelDispatcher; +}; + /** * A wrapper for editor section. Its responsibility is rendering a header, * expanding, collapsing, and removing the section. diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts index d941faac3435c..e15d20963ef75 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts @@ -39,14 +39,17 @@ import { Verb, } from 'teleport/services/resources/types'; +import { RoleEditorModelValidationResult } from './validation'; import { defaultOptions } from './withDefaults'; export type StandardEditorModel = { roleModel: RoleEditorModel; + originalRole: Role; /** * Will be true if fields have been modified from the original. */ isDirty: boolean; + validationResult: RoleEditorModelValidationResult; }; /** diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts new file mode 100644 index 0000000000000..ad778a2c159fc --- /dev/null +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts @@ -0,0 +1,197 @@ +/** + * Teleport + * Copyright (C) 2025 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 { Dispatch, useReducer } from 'react'; + +import { Role } from 'teleport/services/resources'; + +import { + hasModifiedFields, + MetadataModel, + newResourceAccess, + newRole, + newRuleModel, + OptionsModel, + ResourceAccess, + ResourceAccessKind, + RoleEditorModel, + roleToRoleEditorModel, + RuleModel, + StandardEditorModel, +} from './standardmodel'; +import { validateRoleEditorModel } from './validation'; + +/** + * Creates a standard model state and returns an array composed of the state + * and an action dispatcher that can be used to change it. + */ +export const useStandardModel = ( + originalRole?: Role +): [StandardEditorModel, StandardModelDispatcher] => + useReducer(reduce, originalRole, initializeState); + +const initializeState = (originalRole?: Role): StandardEditorModel => { + const role = originalRole ?? newRole(); + const roleModel = roleToRoleEditorModel(role, role); + return { + roleModel, + originalRole, + isDirty: !originalRole, // New role is dirty by default. + validationResult: validateRoleEditorModel(roleModel, undefined, undefined), + }; +}; + +/** A function for dispatching model-changing actions. */ +export type StandardModelDispatcher = Dispatch; + +type StandardModelAction = + | SetModelAction + | SetMetadataAction + | AddResourceAccessAction + | SetResourceAccessAction + | RemoveResourceAccessAction + | AddAccessRuleAction + | SetAccessRuleAction + | RemoveAccessRuleAction + | SetOptionsAction; + +/** Sets the entire model. */ +type SetModelAction = { type: 'set-role-model'; payload: RoleEditorModel }; +type SetMetadataAction = { type: 'set-metadata'; payload: MetadataModel }; +type AddResourceAccessAction = { + type: 'add-resource-access'; + payload: { kind: ResourceAccessKind }; +}; +type SetResourceAccessAction = { + type: 'set-resource-access'; + payload: ResourceAccess; +}; +type RemoveResourceAccessAction = { + type: 'remove-resource-access'; + payload: { kind: ResourceAccessKind }; +}; +type AddAccessRuleAction = { type: 'add-access-rule'; payload?: never }; +type SetAccessRuleAction = { type: 'set-access-rule'; payload: RuleModel }; +type RemoveAccessRuleAction = { + type: 'remove-access-rule'; + payload: { id: string }; +}; +type SetOptionsAction = { type: 'set-options'; payload: OptionsModel }; + +/** Produces a new model using existing state and the action. */ +const reduce = ( + state: StandardEditorModel, + action: StandardModelAction +): StandardEditorModel => { + // We need to give `type` a different name or the assertion in the `default` + // block will cause a syntax error. + const { type, payload } = action; + + switch (type) { + case 'set-role-model': + return updateRoleModel(state, payload); + + case 'set-metadata': + return patchRoleModel(state, { metadata: payload }); + + case 'set-options': + return patchRoleModel(state, { options: payload }); + + case 'add-resource-access': + return patchRoleModel(state, { + resources: [ + ...state.roleModel.resources, + newResourceAccess(payload.kind), + ], + }); + + case 'set-resource-access': + return patchRoleModel(state, { + resources: state.roleModel.resources.map(r => + r.kind === payload.kind ? payload : r + ), + }); + + case 'remove-resource-access': + return patchRoleModel(state, { + resources: state.roleModel.resources.filter( + r => r.kind !== payload.kind + ), + }); + + case 'add-access-rule': + return patchRoleModel(state, { + rules: [...state.roleModel.rules, newRuleModel()], + }); + + case 'set-access-rule': + return patchRoleModel(state, { + rules: state.roleModel.rules.map(r => + r.id === payload.id ? payload : r + ), + }); + + case 'remove-access-rule': + return patchRoleModel(state, { + rules: state.roleModel.rules.filter(r => r.id !== payload.id), + }); + + default: + (type) satisfies never; + return state; + } +}; + +const patchRoleModel = ( + { roleModel, originalRole, validationResult }: StandardEditorModel, + roleModelPatch: Partial +): StandardEditorModel => { + const newRoleModel = { ...roleModel, ...roleModelPatch }; + return { + roleModel: newRoleModel, + originalRole, + isDirty: hasModifiedFields(newRoleModel, originalRole), + validationResult: validateRoleEditorModel( + newRoleModel, + roleModel, + validationResult + ), + }; +}; + +/** + * Creates a new model state given existing state and a patch to + * RoleEditorModel. Validates the state and recognizes whether it's dirty (i.e. + * changed from the original). + */ +const updateRoleModel = ( + { roleModel, originalRole, validationResult }: StandardEditorModel, + roleModelPatch: Partial +): StandardEditorModel => { + const newRoleModel = { ...roleModel, ...roleModelPatch }; + return { + roleModel: newRoleModel, + originalRole, + isDirty: hasModifiedFields(newRoleModel, originalRole), + validationResult: validateRoleEditorModel( + newRoleModel, + roleModel, + validationResult + ), + }; +}; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts new file mode 100644 index 0000000000000..791a9ce8d9f05 --- /dev/null +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts @@ -0,0 +1,167 @@ +/** + * Teleport + * Copyright (C) 2025 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 { ResourceKind } from 'teleport/services/resources'; + +import { + ResourceAccess, + roleToRoleEditorModel, + RuleModel, +} from './standardmodel'; +import { + validateAccessRule, + validateResourceAccess, + validateRoleEditorModel, +} from './validation'; +import { withDefaults } from './withDefaults'; + +const minimalRoleModel = () => + roleToRoleEditorModel( + withDefaults({ metadata: { name: 'role-name' }, version: 'v7' }) + ); + +const validity = (arr: { valid: boolean }[]) => arr.map(it => it.valid); + +describe('validateRoleEditorModel', () => { + test('valid minimal model', () => { + const result = validateRoleEditorModel( + minimalRoleModel(), + undefined, + undefined + ); + expect(result.metadata.valid).toBe(true); + expect(result.resources).toEqual([]); + expect(result.rules).toEqual([]); + }); + + test('valid complex model', () => { + const model = minimalRoleModel(); + model.metadata.labels = [{ name: 'foo', value: 'bar' }]; + model.resources = [ + { + kind: 'kube_cluster', + labels: [{ name: 'foo', value: 'bar' }], + groups: [], + resources: [ + { + id: 'dummy-id', + kind: { label: 'pod', value: 'pod' }, + name: 'res-name', + namespace: 'dummy-namespace', + verbs: [], + }, + ], + }, + { + kind: 'node', + labels: [{ name: 'foo', value: 'bar' }], + logins: [{ label: 'root', value: 'root' }], + }, + { + kind: 'app', + labels: [{ name: 'foo', value: 'bar' }], + awsRoleARNs: ['some-arn'], + azureIdentities: ['some-azure-id'], + gcpServiceAccounts: ['some-gcp-acct'], + }, + { + kind: 'db', + labels: [{ name: 'foo', value: 'bar' }], + roles: [{ label: 'some-role', value: 'some-role' }], + names: [], + users: [], + }, + { + kind: 'windows_desktop', + labels: [{ name: 'foo', value: 'bar' }], + logins: [], + }, + ]; + model.rules = [ + { + id: 'dummy-id', + resources: [{ label: ResourceKind.Node, value: ResourceKind.Node }], + verbs: [{ label: '*', value: '*' }], + }, + ]; + const result = validateRoleEditorModel(model, undefined, undefined); + expect(result.metadata.valid).toBe(true); + expect(validity(result.resources)).toEqual([true, true, true, true, true]); + expect(validity(result.rules)).toEqual([true]); + }); + + test('invalid metadata', () => { + const model = minimalRoleModel(); + model.metadata.name = ''; + const result = validateRoleEditorModel(model, undefined, undefined); + expect(result.metadata.valid).toBe(false); + }); + + test('invalid resource', () => { + const model = minimalRoleModel(); + model.resources = [ + { + kind: 'node', + labels: [{ name: 'foo', value: '' }], + logins: [], + }, + ]; + const result = validateRoleEditorModel(model, undefined, undefined); + expect(validity(result.resources)).toEqual([false]); + }); + + test('invalid access rule', () => { + const model = minimalRoleModel(); + model.rules = [ + { + id: 'dummy-id', + resources: [], + verbs: [{ label: '*', value: '*' }], + }, + ]; + const result = validateRoleEditorModel(model, undefined, undefined); + expect(validity(result.rules)).toEqual([false]); + }); + + it('reuses previously computed section results', () => { + const model = minimalRoleModel(); + const result1 = validateRoleEditorModel(model, undefined, undefined); + const result2 = validateRoleEditorModel(model, model, result1); + expect(result2.metadata).toBe(result1.metadata); + expect(result2.resources).toBe(result1.resources); + expect(result2.rules).toBe(result1.rules); + }); +}); + +describe('validateResourceAccess', () => { + it('reuses previously computed results', () => { + const resource: ResourceAccess = { kind: 'node', labels: [], logins: [] }; + const result1 = validateResourceAccess(resource, undefined, undefined); + const result2 = validateResourceAccess(resource, resource, result1); + expect(result2).toBe(result1); + }); +}); + +describe('validateAccessRule', () => { + it('reuses previously computed results', () => { + const rule: RuleModel = { id: 'some-id', resources: [], verbs: [] }; + const result1 = validateAccessRule(rule, undefined, undefined); + const result2 = validateAccessRule(rule, rule, result1); + expect(result2).toBe(result1); + }); +}); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts index 2e4d5865ecd95..f9c678dfc77c2 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts @@ -45,19 +45,60 @@ const kubernetesClusterWideResourceKinds: KubernetesResourceKind[] = [ 'certificatesigningrequest', ]; -export function validateRoleEditorModel({ - metadata, - resources, - rules, -}: RoleEditorModel) { +export type RoleEditorModelValidationResult = { + metadata: MetadataValidationResult; + resources: ResourceAccessValidationResult[]; + rules: AccessRuleValidationResult[]; +}; + +/** + * Validates the role editor model. In addition to the model itself, this + * function also takes the previous model and previous validation result. The + * intention here is to only return a newly created result if the previous + * model is indeed different. This pattern is then repeated in other validation + * functions. + * + * The purpose of this is less about the performance of the validation process + * itself, and more about enabling memoization-based rendering optimizations: + * UI components that take either entire or partial validation results can be + * cached if the validation results don't changed. + * + * Note that we can't use `useMemo` here, because `validateRoleEditorModel` is + * called from the state reducer. Also `highbar.memoize` was not applicable, as + * it caches an arbitrary amount of previous results. + */ +export function validateRoleEditorModel( + model: RoleEditorModel, + previousModel: RoleEditorModel | undefined, + previousResult: RoleEditorModelValidationResult | undefined +): RoleEditorModelValidationResult { return { - metadata: validateMetadata(metadata), - resources: resources.map(validateResourceAccess), - rules: rules.map(validateAccessRule), + metadata: validateMetadata( + model.metadata, + previousModel?.metadata, + previousResult?.metadata + ), + resources: validateResourceAccessList( + model.resources, + previousModel?.resources, + previousResult?.resources + ), + rules: validateAccessRuleList( + model.rules, + previousModel?.rules, + previousResult?.rules + ), }; } -function validateMetadata(model: MetadataModel): MetadataValidationResult { +function validateMetadata( + model: MetadataModel, + previousModel: MetadataModel, + previousResult: MetadataValidationResult +): MetadataValidationResult { + if (previousModel === model) { + return previousResult; + } return runRules(model, metadataRules); } @@ -69,21 +110,40 @@ export type MetadataValidationResult = RuleSetValidationResult< typeof metadataRules >; +function validateResourceAccessList( + resources: ResourceAccess[], + previousResources: ResourceAccess[], + previousResults: ResourceAccessValidationResult[] +): ResourceAccessValidationResult[] { + if (previousResources === resources) { + return previousResults; + } + return resources.map((res, i) => + validateResourceAccess(res, previousResources?.[i], previousResults?.[i]) + ); +} + export function validateResourceAccess( - res: ResourceAccess + resource: ResourceAccess, + previousResource: ResourceAccess, + previousResult: ResourceAccessValidationResult ): ResourceAccessValidationResult { - const { kind } = res; + if (resource === previousResource) { + return previousResult; + } + + const { kind } = resource; switch (kind) { case 'kube_cluster': - return runRules(res, kubernetesAccessValidationRules); + return runRules(resource, kubernetesAccessValidationRules); case 'node': - return runRules(res, serverAccessValidationRules); + return runRules(resource, serverAccessValidationRules); case 'app': - return runRules(res, appAccessValidationRules); + return runRules(resource, appAccessValidationRules); case 'db': - return runRules(res, databaseAccessValidationRules); + return runRules(resource, databaseAccessValidationRules); case 'windows_desktop': - return runRules(res, windowsDesktopAccessValidationRules); + return runRules(resource, windowsDesktopAccessValidationRules); default: kind satisfies never; } @@ -171,8 +231,29 @@ export type WindowsDesktopAccessValidationResult = RuleSetValidationResult< typeof windowsDesktopAccessValidationRules >; -export const validateAccessRule = (accessRule: RuleModel) => - runRules(accessRule, accessRuleValidationRules); +export function validateAccessRuleList( + rules: RuleModel[], + previousRules: RuleModel[], + previousResults: AccessRuleValidationResult[] +): AccessRuleValidationResult[] { + if (previousRules === rules) { + return previousResults; + } + return rules.map((rule, i) => + validateAccessRule(rule, previousRules?.[i], previousResults?.[i]) + ); +} + +export const validateAccessRule = ( + rule: RuleModel, + previousRule: RuleModel, + previousResult: AccessRuleValidationResult +): AccessRuleValidationResult => { + if (previousRule === rule) { + return previousResult; + } + return runRules(rule, accessRuleValidationRules); +}; const accessRuleValidationRules = { resources: requiredField('At least one resource kind is required'),