diff --git a/package.json b/package.json index 7bc300198a9..01db8b2844f 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "d3-time-format": "^4.1.0", "date-fns": "^2.28.0", "history": "^4.9.0", + "immer": "^10.1.1", "prop-types": "^15.8.1", "react": "^18.3.1", "react-day-picker": "^8.10.1", @@ -95,6 +96,7 @@ "react-transition-group": "^4.4.5", "styled-components": "^6.1.13", "tslib": "^2.8.1", + "use-immer": "^0.11.0", "whatwg-fetch": "^3.6.20" }, "msw": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 83e985448ad..1fa010c37c8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -55,6 +55,9 @@ importers: history: specifier: ^4.9.0 version: 4.10.1 + immer: + specifier: ^10.1.1 + version: 10.1.1 prop-types: specifier: ^15.8.1 version: 15.8.1 @@ -91,6 +94,9 @@ importers: tslib: specifier: ^2.8.1 version: 2.8.1 + use-immer: + specifier: ^0.11.0 + version: 0.11.0(immer@10.1.1)(react@18.3.1) whatwg-fetch: specifier: ^3.6.20 version: 3.6.20 @@ -484,9 +490,6 @@ importers: events: specifier: 3.3.0 version: 3.3.0 - immer: - specifier: ^10.1.1 - version: 10.1.1 jest-canvas-mock: specifier: ^2.5.2 version: 2.5.2 @@ -3579,8 +3582,8 @@ packages: convert-source-map@2.0.0: resolution: {integrity: sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==} - cookie@0.7.2: - resolution: {integrity: sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==} + cookie@0.7.1: + resolution: {integrity: sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==} engines: {node: '>= 0.6'} core-js-compat@3.38.1: @@ -6645,6 +6648,12 @@ packages: url-parse@1.5.10: resolution: {integrity: sha512-WypcfiRhfeUP9vvF0j6rw0J3hrWrw6iZv3+22h6iRMJ/8z1Tj6XfLP4DsUix5MhMPnXpiHDoKyoZ/bdCkwBCiQ==} + use-immer@0.11.0: + resolution: {integrity: sha512-RNAqi3GqsWJ4bcCd4LMBgdzvPmTABam24DUaFiKfX9s3MSorNRz9RDZYJkllJoMHUxVLMDetwAuCDeyWNrp1yA==} + peerDependencies: + immer: '>=8.0.0' + react: ^16.8.0 || ^17.0.1 || ^18.0.0 || ^19.0.0 + use-isomorphic-layout-effect@1.2.0: resolution: {integrity: sha512-q6ayo8DWoPZT0VdG4u3D3uxcgONP3Mevx2i2b0434cwWBoL+aelL1DzkXI6w3PhTZzUeR2kaVlZn70iCiseP6w==} peerDependencies: @@ -7791,7 +7800,7 @@ snapshots: '@bundled-es-modules/cookie@2.0.1': dependencies: - cookie: 0.7.2 + cookie: 0.7.1 '@bundled-es-modules/statuses@1.0.1': dependencies: @@ -10601,7 +10610,7 @@ snapshots: convert-source-map@2.0.0: {} - cookie@0.7.2: {} + cookie@0.7.1: {} core-js-compat@3.38.1: dependencies: @@ -14372,6 +14381,11 @@ snapshots: querystringify: 2.2.0 requires-port: 1.0.0 + use-immer@0.11.0(immer@10.1.1)(react@18.3.1): + dependencies: + immer: 10.1.1 + react: 18.3.1 + use-isomorphic-layout-effect@1.2.0(@types/react@18.3.12)(react@18.3.1): dependencies: react: 18.3.1 diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx index c85c6449eb4..78cec16648d 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx @@ -27,6 +27,7 @@ import Flex from 'design/Flex'; import useResources from 'teleport/components/useResources'; import cfg from 'teleport/config'; import { createTeleportContext } from 'teleport/mocks/contexts'; +import { RoleVersion } from 'teleport/services/resources'; import { Access } from 'teleport/services/user'; import { YamlSupportedResourceKind } from 'teleport/services/yaml/types'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; @@ -73,7 +74,7 @@ const parseHandler = http.post( HttpResponse.json({ resource: withDefaults({ metadata: { name: 'dummy-role' }, - version: 'v7', + version: RoleVersion.V7, }), }) ); diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx index 2b1787df44e..40ff87b3ba6 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx @@ -32,6 +32,7 @@ import { import TeleportContextProvider from 'teleport/TeleportContextProvider'; import { RoleEditor, RoleEditorProps } from './RoleEditor'; +import { defaultRoleVersion } from './StandardEditor/standardmodel'; import { defaultOptions, withDefaults } from './StandardEditor/withDefaults'; // The Ace editor is very difficult to deal with in tests, especially that for @@ -95,7 +96,7 @@ test('rendering and switching tabs for new role', async () => { deny: {}, options: {}, }, - version: 'v7', + version: defaultRoleVersion, }) ); expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/MetadataSection.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/MetadataSection.tsx index eed7048b54a..fd2914fb483 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/MetadataSection.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/MetadataSection.tsx @@ -16,14 +16,16 @@ * along with this program. If not, see . */ +import Box from 'design/Box'; import Text from 'design/Text'; import FieldInput from 'shared/components/FieldInput'; +import { FieldSelect } from 'shared/components/FieldSelect'; import { precomputed } from 'shared/components/Validation/rules'; import { LabelsInput } from 'teleport/components/LabelsInput'; import { SectionBox, SectionProps } from './sections'; -import { MetadataModel } from './standardmodel'; +import { MetadataModel, roleVersionOptions } from './standardmodel'; import { MetadataValidationResult } from './validation'; export const MetadataSection = ({ @@ -55,14 +57,24 @@ export const MetadataSection = ({ onChange({ ...value, description: e.target.value }) } /> - - Labels - - onChange?.({ ...value, labels })} - rule={precomputed(validation.fields.labels)} + + + Labels + + onChange({ ...value, labels })} + rule={precomputed(validation.fields.labels)} + /> + + onChange({ ...value, version })} + mb={0} /> ); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.test.tsx index 2c57c2c7efc..62ff7b15997 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.test.tsx @@ -22,6 +22,8 @@ import selectEvent from 'react-select-event'; import { render, screen, userEvent } from 'design/utils/testing'; import { Validator } from 'shared/components/Validation'; +import { RoleVersion } from 'teleport/services/resources'; + import { AppAccessSection, DatabaseAccessSection, @@ -32,6 +34,7 @@ import { import { AppAccess, DatabaseAccess, + defaultRoleVersion, KubernetesAccess, newResourceAccess, ServerAccess, @@ -50,7 +53,7 @@ describe('ServerAccessSection', () => { render( component={ServerAccessSection} - defaultValue={newResourceAccess('node')} + defaultValue={newResourceAccess('node', defaultRoleVersion)} onChange={onChange} validatorRef={v => { validator = v; @@ -104,13 +107,16 @@ describe('ServerAccessSection', () => { }); describe('KubernetesAccessSection', () => { - const setup = () => { + const setup = (roleVersion: RoleVersion = defaultRoleVersion) => { const onChange = jest.fn(); let validator: Validator; render( component={KubernetesAccessSection} - defaultValue={newResourceAccess('kube_cluster')} + defaultValue={{ + ...newResourceAccess('kube_cluster', defaultRoleVersion), + roleVersion, + }} onChange={onChange} validatorRef={v => { validator = v; @@ -169,8 +175,10 @@ describe('KubernetesAccessSection', () => { expect.objectContaining({ value: 'create' }), expect.objectContaining({ value: 'delete' }), ], + roleVersion: 'v7', }, ], + roleVersion: 'v7', } as KubernetesAccess); }); @@ -228,12 +236,20 @@ describe('KubernetesAccessSection', () => { }); test('validation', async () => { - const { user, validator } = setup(); + const { user, validator } = setup(RoleVersion.V6); await user.click(screen.getByRole('button', { name: 'Add a Label' })); await user.click(screen.getByRole('button', { name: 'Add a Resource' })); + await selectEvent.select(screen.getByLabelText('Kind'), 'Service'); await user.clear(screen.getByLabelText('Name')); await user.clear(screen.getByLabelText('Namespace')); + await selectEvent.select(screen.getByLabelText('Verbs'), [ + 'All verbs', + 'create', + ]); act(() => validator.validate()); + expect( + screen.getByText('Only pods are allowed for role version v6') + ).toBeVisible(); expect( screen.getByPlaceholderText('label key') ).toHaveAccessibleDescription('required'); @@ -243,6 +259,9 @@ describe('KubernetesAccessSection', () => { expect(screen.getByLabelText('Namespace')).toHaveAccessibleDescription( 'Namespace is required for resources of this kind' ); + expect( + screen.getByText('Mixing "All verbs" with other options is not allowed') + ).toBeVisible(); }); }); @@ -253,7 +272,7 @@ describe('AppAccessSection', () => { render( component={AppAccessSection} - defaultValue={newResourceAccess('app')} + defaultValue={newResourceAccess('app', defaultRoleVersion)} onChange={onChange} validatorRef={v => { validator = v; @@ -359,7 +378,7 @@ describe('DatabaseAccessSection', () => { render( component={DatabaseAccessSection} - defaultValue={newResourceAccess('db')} + defaultValue={newResourceAccess('db', defaultRoleVersion)} onChange={onChange} validatorRef={v => { validator = v; @@ -425,7 +444,7 @@ describe('WindowsDesktopAccessSection', () => { render( component={WindowsDesktopAccessSection} - defaultValue={newResourceAccess('windows_desktop')} + defaultValue={newResourceAccess('windows_desktop', defaultRoleVersion)} onChange={onChange} validatorRef={v => { validator = v; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx index 5090d7ac6ba..5b59d78b35b 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx @@ -28,7 +28,8 @@ import { Mark } from 'design/Mark'; import Text, { H4 } from 'design/Text'; import FieldInput from 'shared/components/FieldInput'; import { FieldMultiInput } from 'shared/components/FieldMultiInput/FieldMultiInput'; -import FieldSelect, { +import { + FieldSelect, FieldSelectCreatable, } from 'shared/components/FieldSelect'; import { MenuButton, MenuItem } from 'shared/components/MenuAction'; @@ -321,7 +322,10 @@ export function KubernetesAccessSection({ onClick={() => onChange?.({ ...value, - resources: [...value.resources, newKubernetesResourceModel()], + resources: [ + ...value.resources, + newKubernetesResourceModel(value.roleVersion), + ], }) } > @@ -378,6 +382,7 @@ function KubernetesResourceView({ isDisabled={isProcessing} options={kubernetesResourceKindOptions} value={kind} + rule={precomputed(validation.kind)} onChange={k => onChange?.({ ...value, kind: k })} /> onChange?.({ ...value, verbs: v })} mb={0} /> 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 95ebd826670..84dde93a3aa 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx @@ -138,8 +138,10 @@ test('edits metadata', async () => { const onSave = (r: Role) => (role = r); render(); await user.type(screen.getByLabelText('Description'), 'foo'); + await selectEvent.select(screen.getByLabelText('Version'), 'v6'); await user.click(screen.getByRole('button', { name: 'Create Role' })); expect(role.metadata.description).toBe('foo'); + expect(role.version).toBe('v6'); }); test('edits resource access', async () => { @@ -159,6 +161,41 @@ test('edits resource access', async () => { expect(role.spec.allow.logins).toEqual(['{{internal.logins}}', 'ec2-user']); }); +test('triggers v6 validation for Kubernetes resources', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + render(); + await selectEvent.select(screen.getByLabelText('Version'), 'v6'); + 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: 'Kubernetes' })); + await user.click(screen.getByRole('button', { name: 'Add a Resource' })); + await selectEvent.select(screen.getByLabelText('Kind'), 'Job'); + + // Adding a second resource to make sure that we don't run into attempting to + // modify an immer-frozen object. This might happen if the reducer tried to + // modify resources that were already there. + await user.click( + screen.getByRole('button', { name: 'Add Another Resource' }) + ); + await selectEvent.select(screen.getAllByLabelText('Kind')[1], 'Pod'); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + + // Validation should have failed on a Job resource and role v6. + expect( + screen.getByText('Only pods are allowed for role version v6') + ).toBeVisible(); + expect(onSave).not.toHaveBeenCalled(); + + // Back to v7, try again + await user.click(screen.getByRole('tab', { name: 'Overview' })); + await selectEvent.select(screen.getByLabelText('Version'), 'v7'); + await user.click(screen.getByRole('button', { name: 'Create Role' })); + expect(onSave).toHaveBeenCalled(); +}); + const getAllMenuItemNames = () => screen.queryAllByRole('menuitem').map(m => m.textContent); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx index 387378909b8..87780b97ac9 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StatefulSection.tsx @@ -21,7 +21,7 @@ import React, { useReducer } from 'react'; import Validation, { Validator } from 'shared/components/Validation'; import { SectionProps, SectionPropsWithDispatch } from './sections'; -import { StandardEditorModel } from './standardmodel'; +import { defaultRoleVersion, StandardEditorModel } from './standardmodel'; import { useStandardModel } from './useStandardModel'; import { withDefaults } from './withDefaults'; @@ -78,7 +78,7 @@ export function StatefulSection({ const minimalRole = withDefaults({ metadata: { name: 'foobar' }, - version: 'v7', + version: defaultRoleVersion, }); /** A helper for testing editor section components. */ diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx index 272affddc51..8d099c5aa61 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx @@ -35,7 +35,7 @@ export type SectionProps = { value: Model; isProcessing: boolean; validation?: ValidationResult; - onChange?(value: Model): void; + onChange(value: Model): void; }; /** Properties of a section that uses a dispatcher to change the model. */ diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts index 1d8ffeb387a..91d96b16380 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts @@ -25,6 +25,7 @@ import { RequireMFAType, ResourceKind, Role, + RoleVersion, Rule, SessionRecordingMode, SSHPortForwarding, @@ -33,6 +34,7 @@ import { import { createDBUserModeOptionsMap, createHostUserModeOptionsMap, + defaultRoleVersion, KubernetesAccess, kubernetesResourceKindOptionsMap, kubernetesVerbOptionsMap, @@ -44,6 +46,7 @@ import { RoleEditorModel, roleEditorModelToRole, roleToRoleEditorModel, + roleVersionOptionsMap, sessionRecordingModeOptionsMap, sshPortForwardingModeOptionsMap, verbOptionsMap, @@ -51,10 +54,14 @@ import { import { optionsWithDefaults, withDefaults } from './withDefaults'; const minimalRole = () => - withDefaults({ metadata: { name: 'foobar' }, version: 'v7' }); + withDefaults({ metadata: { name: 'foobar' }, version: defaultRoleVersion }); const minimalRoleModel = (): RoleEditorModel => ({ - metadata: { name: 'foobar', labels: [] }, + metadata: { + name: 'foobar', + labels: [], + version: roleVersionOptionsMap.get(defaultRoleVersion), + }, resources: [], rules: [], requiresReset: false, @@ -92,6 +99,7 @@ describe.each<{ name: string; role: Role; model: RoleEditorModel }>([ description: 'role-description', labels: { foo: 'bar' }, }, + version: RoleVersion.V6, }, model: { ...minimalRoleModel(), @@ -99,6 +107,7 @@ describe.each<{ name: string; role: Role; model: RoleEditorModel }>([ name: 'role-name', description: 'role-description', labels: [{ name: 'foo', value: 'bar' }], + version: roleVersionOptionsMap.get(RoleVersion.V6), }, }, }, @@ -431,6 +440,7 @@ describe('roleToRoleEditorModel', () => { groups: [], labels: [], resources: [], + roleVersion: defaultRoleVersion, }); test.each<{ name: string; role: Role; model?: RoleEditorModel }>([ @@ -767,8 +777,10 @@ describe('roleToRoleEditorModel', () => { } ); - test('version change requires reset', () => { - expect(roleToRoleEditorModel({ ...minimalRole(), version: 'v1' })).toEqual({ + test('unsupported version requires reset', () => { + expect( + roleToRoleEditorModel({ ...minimalRole(), version: 'v1' as RoleVersion }) + ).toEqual({ ...minimalRoleModel(), requiresReset: true, } as RoleEditorModel); @@ -795,6 +807,7 @@ describe('roleToRoleEditorModel', () => { name: 'role-name', revision: originalRev, labels: [], + version: roleVersionOptionsMap.get(defaultRoleVersion), }, requiresReset: true, } as RoleEditorModel); @@ -824,6 +837,7 @@ describe('roleToRoleEditorModel', () => { name: 'role-name', revision: 'e39ea9f1-79b7-4d28-8f0c-af6848f9e655', labels: [], + version: roleVersionOptionsMap.get(defaultRoleVersion), }, requiresReset: true, } as RoleEditorModel); @@ -877,6 +891,7 @@ describe('roleToRoleEditorModel', () => { kubernetesVerbOptionsMap.get('get'), kubernetesVerbOptionsMap.get('update'), ], + roleVersion: defaultRoleVersion, }, { id: expect.any(String), @@ -884,8 +899,10 @@ describe('roleToRoleEditorModel', () => { name: 'some-node', namespace: '', verbs: [], + roleVersion: defaultRoleVersion, }, ], + roleVersion: defaultRoleVersion, }, ], } as RoleEditorModel); @@ -974,6 +991,7 @@ describe('roleEditorModelToRole', () => { description: 'walks dogs', revision: 'e2a3ccf8-09b9-4d97-8e47-6dbe3d53c0e5', labels: [{ name: 'kind', value: 'occupation' }], + version: roleVersionOptionsMap.get(RoleVersion.V5), }, }) ).toEqual({ @@ -984,6 +1002,7 @@ describe('roleEditorModelToRole', () => { revision: 'e2a3ccf8-09b9-4d97-8e47-6dbe3d53c0e5', labels: { kind: 'occupation' }, }, + version: 'v5', } as Role); }); @@ -1012,6 +1031,7 @@ describe('roleEditorModelToRole', () => { kubernetesVerbOptionsMap.get('get'), kubernetesVerbOptionsMap.get('update'), ], + roleVersion: defaultRoleVersion, }, { id: 'dummy-id-2', @@ -1019,8 +1039,10 @@ describe('roleEditorModelToRole', () => { name: 'some-node', namespace: '', verbs: [], + roleVersion: defaultRoleVersion, }, ], + roleVersion: defaultRoleVersion, }, ], }) diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts index 4aa46945feb..e0814f07310 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts @@ -34,6 +34,7 @@ import { RequireMFAType, ResourceKind, RoleOptions, + RoleVersion, Rule, SessionRecordingMode, SSHPortForwarding, @@ -77,6 +78,7 @@ export type MetadataModel = { description?: string; revision?: string; labels: UILabel[]; + version: RoleVersionOption; }; /** A model for resource section. */ @@ -112,6 +114,14 @@ export type KubernetesAccess = ResourceAccessBase<'kube_cluster'> & { groups: readonly Option[]; labels: UILabel[]; resources: KubernetesResourceModel[]; + + /** + * Version of the role that owns this section. Required to propagate it to + * {@link KubernetesResourceModel}. It's the responsibility of + * `useStandardModel` reducer to keep this value consistent with the actual + * role version. + */ + roleVersion: RoleVersion; }; export type KubernetesResourceModel = { @@ -121,6 +131,14 @@ export type KubernetesResourceModel = { name: string; namespace: string; verbs: readonly KubernetesVerbOption[]; + + /** + * Version of the role that owns this section. Required in order to support + * version-specific validation rules. It's the responsibility of + * `useStandardModel` reducer to keep this value consistent with the actual + * role version. + */ + roleVersion: RoleVersion; }; type KubernetesResourceKindOption = Option; @@ -171,7 +189,7 @@ export const kubernetesResourceKindOptionsMap = optionsToMap( kubernetesResourceKindOptions ); -type KubernetesVerbOption = Option; +export type KubernetesVerbOption = Option; /** * All possible Kubernetes verb drop-down options. This array needs to be kept * in sync with `KubernetesVerbs` in `api/types/constants.go. @@ -361,7 +379,14 @@ export const sshPortForwardingModeOptionsMap = optionsToMap( sshPortForwardingModeOptions ); -const roleVersion = 'v7'; +export type RoleVersionOption = Option; +export const roleVersionOptions = Object.values(RoleVersion) + .toSorted() + .toReversed() + .map(o => ({ value: o, label: o })); +export const roleVersionOptionsMap = optionsToMap(roleVersionOptions); + +export const defaultRoleVersion = RoleVersion.V7; /** * Returns the role object with required fields defined with empty values. @@ -377,19 +402,44 @@ export function newRole(): Role { deny: {}, options: defaultOptions(), }, - version: roleVersion, + version: defaultRoleVersion, }; } -export function newResourceAccess(kind: 'node'): ServerAccess; -export function newResourceAccess(kind: 'kube_cluster'): KubernetesAccess; -export function newResourceAccess(kind: 'app'): AppAccess; -export function newResourceAccess(kind: 'db'): DatabaseAccess; export function newResourceAccess( - kind: 'windows_desktop' + kind: 'node', + roleVersion: RoleVersion +): ServerAccess; + +export function newResourceAccess( + kind: 'kube_cluster', + roleVersion: RoleVersion +): KubernetesAccess; + +export function newResourceAccess( + kind: 'app', + roleVersion: RoleVersion +): AppAccess; + +export function newResourceAccess( + kind: 'db', + roleVersion: RoleVersion +): DatabaseAccess; + +export function newResourceAccess( + kind: 'windows_desktop', + roleVersion: RoleVersion ): WindowsDesktopAccess; -export function newResourceAccess(kind: ResourceAccessKind): AppAccess; -export function newResourceAccess(kind: ResourceAccessKind): ResourceAccess { + +export function newResourceAccess( + kind: ResourceAccessKind, + roleVersion: RoleVersion +): AppAccess; + +export function newResourceAccess( + kind: ResourceAccessKind, + roleVersion: RoleVersion +): ResourceAccess { switch (kind) { case 'node': return { @@ -403,6 +453,7 @@ export function newResourceAccess(kind: ResourceAccessKind): ResourceAccess { groups: [stringToOption('{{internal.kubernetes_groups}}')], labels: [], resources: [], + roleVersion, }; case 'app': return { @@ -431,13 +482,16 @@ export function newResourceAccess(kind: ResourceAccessKind): ResourceAccess { } } -export function newKubernetesResourceModel(): KubernetesResourceModel { +export function newKubernetesResourceModel( + roleVersion: RoleVersion +): KubernetesResourceModel { return { id: crypto.randomUUID(), kind: kubernetesResourceKindOptions.find(k => k.value === '*'), name: '*', namespace: '*', verbs: [], + roleVersion, }; } @@ -470,9 +524,10 @@ export function roleToRoleEditorModel( resources, rules, requiresReset: allowRequiresReset, - } = roleConditionsToModel(allow); + } = roleConditionsToModel(allow, version); const { model: optionsModel, requiresReset: optionsRequireReset } = optionsToModel(options); + const versionOption = roleVersionOptionsMap.get(version); return { metadata: { @@ -480,13 +535,14 @@ export function roleToRoleEditorModel( description, revision: originalRole?.metadata?.revision, labels: labelsToModel(labels), + version: versionOption ?? roleVersionOptionsMap.get(RoleVersion.V7), }, resources, rules, options: optionsModel, requiresReset: revision !== originalRole?.metadata?.revision || - version !== roleVersion || + versionOption === undefined || !( isEmpty(unsupported) && isEmpty(unsupportedMetadata) && @@ -503,7 +559,8 @@ export function roleToRoleEditorModel( * specific) to a part of the role editor model. */ function roleConditionsToModel( - conditions: RoleConditions + conditions: RoleConditions, + roleVersion: RoleVersion ): Pick { const { node_labels, @@ -548,13 +605,14 @@ function roleConditionsToModel( const { model: kubeResourcesModel, requiresReset: kubernetesResourcesRequireReset, - } = kubernetesResourcesToModel(kubernetes_resources); + } = kubernetesResourcesToModel(kubernetes_resources, roleVersion); if (someNonEmpty(kubeGroupsModel, kubeLabelsModel, kubeResourcesModel)) { resources.push({ kind: 'kube_cluster', groups: kubeGroupsModel, labels: kubeLabelsModel, resources: kubeResourcesModel, + roleVersion, }); } @@ -649,16 +707,22 @@ function stringsToOptions(arr: T[]): Option[] { } function kubernetesResourcesToModel( - resources: KubernetesResource[] | undefined + resources: KubernetesResource[] | undefined, + roleVersion: RoleVersion ): { model: KubernetesResourceModel[]; requiresReset: boolean } { - const result = (resources ?? []).map(kubernetesResourceToModel); + const result = (resources ?? []).map(res => + kubernetesResourceToModel(res, roleVersion) + ); return { model: result.map(r => r.model).filter(m => m !== undefined), requiresReset: result.some(r => r.requiresReset), }; } -function kubernetesResourceToModel(res: KubernetesResource): { +function kubernetesResourceToModel( + res: KubernetesResource, + roleVersion: RoleVersion +): { model?: KubernetesResourceModel; requiresReset: boolean; } { @@ -675,6 +739,7 @@ function kubernetesResourceToModel(res: KubernetesResource): { name, namespace, verbs: knownVerbOptions, + roleVersion, } : undefined, requiresReset: @@ -862,7 +927,8 @@ function isEmpty(obj: object) { * Converts a role editor model to a role. This operation is lossless. */ export function roleEditorModelToRole(roleModel: RoleEditorModel): Role { - const { name, description, revision, labels, ...mRest } = roleModel.metadata; + const { name, description, revision, labels, version, ...mRest } = + roleModel.metadata; // Compile-time assert that protects us from silently losing fields. mRest satisfies Record; @@ -882,7 +948,7 @@ export function roleEditorModelToRole(roleModel: RoleEditorModel): Role { deny: {}, options: optionsModelToRoleOptions(roleModel.options), }, - version: roleVersion, + version: version.value, }; for (const res of roleModel.resources) { diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts index c423ea8868c..2d8959602ba 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts @@ -16,9 +16,11 @@ * along with this program. If not, see . */ -import { Dispatch, useReducer } from 'react'; +import { current, original } from 'immer'; +import { Dispatch } from 'react'; +import { useImmerReducer } from 'use-immer'; -import { Role } from 'teleport/services/resources'; +import { Role, RoleVersion } from 'teleport/services/resources'; import { hasModifiedFields, @@ -43,7 +45,7 @@ import { validateRoleEditorModel } from './validation'; export const useStandardModel = ( originalRole?: Role ): [StandardEditorModel, StandardModelDispatcher] => - useReducer(reduce, originalRole, initializeState); + useImmerReducer(reduce, originalRole, initializeState); const initializeState = (originalRole?: Role): StandardEditorModel => { const role = originalRole ?? newRole(); @@ -102,79 +104,93 @@ const reduce = ( // block will cause a syntax error. const { type, payload } = action; + // This reduce uses Immer, so we modify the model draft directly. + // TODO(bl-nero): add immutability to the model data types. switch (type) { case 'set-role-model': - return updateRoleModel(state, payload); + state.roleModel = payload; + break; case 'set-metadata': - return updateRoleModel(state, { metadata: payload }); + state.roleModel.metadata = payload; + updateRoleVersionInResources( + state.roleModel.resources, + payload.version.value + ); + break; case 'set-options': - return updateRoleModel(state, { options: payload }); + state.roleModel.options = payload; + break; case 'add-resource-access': - return updateRoleModel(state, { - resources: [ - ...state.roleModel.resources, - newResourceAccess(payload.kind), - ], - }); + state.roleModel.resources.push( + newResourceAccess(payload.kind, state.roleModel.metadata.version.value) + ); + break; case 'set-resource-access': - return updateRoleModel(state, { - resources: state.roleModel.resources.map(r => - r.kind === payload.kind ? payload : r - ), - }); + state.roleModel.resources = state.roleModel.resources.map(r => + r.kind === payload.kind ? payload : r + ); + break; case 'remove-resource-access': - return updateRoleModel(state, { - resources: state.roleModel.resources.filter( - r => r.kind !== payload.kind - ), - }); + state.roleModel.resources = state.roleModel.resources.filter( + r => r.kind !== payload.kind + ); + break; case 'add-access-rule': - return updateRoleModel(state, { - rules: [...state.roleModel.rules, newRuleModel()], - }); + state.roleModel.rules.push(newRuleModel()); + break; case 'set-access-rule': - return updateRoleModel(state, { - rules: state.roleModel.rules.map(r => - r.id === payload.id ? payload : r - ), - }); + state.roleModel.rules = state.roleModel.rules.map(r => + r.id === payload.id ? payload : r + ); + break; case 'remove-access-rule': - return updateRoleModel(state, { - rules: state.roleModel.rules.filter(r => r.id !== payload.id), - }); + state.roleModel.rules = state.roleModel.rules.filter( + r => r.id !== payload.id + ); + break; default: (type) satisfies never; return state; } + + processEditorModel(state); }; /** - * 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). + * Recomputes dependent fields of a state draft. 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 - ), - }; +const processEditorModel = (state: StandardEditorModel) => { + const { roleModel, originalRole, validationResult } = state; + state.isDirty = hasModifiedFields(roleModel, originalRole); + state.validationResult = validateRoleEditorModel( + // It's crucial to use `current` and `original` here from the performance + // standpoint, since `validateEditorModel` recognizes unchanged state by + // reference. We want to make sure that the objects passed to it have + // stable identities. + current(state).roleModel, + original(state).roleModel, + validationResult + ); +}; + +const updateRoleVersionInResources = ( + resources: ResourceAccess[], + version: RoleVersion +) => { + for (const res of resources.filter(res => res.kind === 'kube_cluster')) { + res.roleVersion = version; + for (const kubeRes of res.resources) { + kubeRes.roleVersion = version; + } + } }; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts index 320aa7f864d..afcdce5ecea 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.test.ts @@ -19,11 +19,16 @@ import { ResourceKind } from 'teleport/services/resources'; import { + defaultRoleVersion, + kubernetesResourceKindOptionsMap, + kubernetesVerbOptionsMap, + newKubernetesResourceModel, ResourceAccess, roleToRoleEditorModel, RuleModel, } from './standardmodel'; import { + KubernetesAccessValidationResult, validateAccessRule, validateResourceAccess, validateRoleEditorModel, @@ -32,7 +37,10 @@ import { withDefaults } from './withDefaults'; const minimalRoleModel = () => roleToRoleEditorModel( - withDefaults({ metadata: { name: 'role-name' }, version: 'v7' }) + withDefaults({ + metadata: { name: 'role-name' }, + version: defaultRoleVersion, + }) ); const validity = (arr: { valid: boolean }[]) => arr.map(it => it.valid); @@ -65,8 +73,10 @@ describe('validateRoleEditorModel', () => { name: 'res-name', namespace: 'dummy-namespace', verbs: [], + roleVersion: defaultRoleVersion, }, ], + roleVersion: defaultRoleVersion, }, { kind: 'node', @@ -129,6 +139,74 @@ describe('validateRoleEditorModel', () => { expect(result.isValid).toBe(false); }); + test('forbids mixing "*" and other Kubernetes verbs', () => { + const model = minimalRoleModel(); + model.resources = [ + { + kind: 'kube_cluster', + groups: [], + labels: [], + resources: [ + { + ...newKubernetesResourceModel(defaultRoleVersion), + verbs: [ + kubernetesVerbOptionsMap.get('*'), + kubernetesVerbOptionsMap.get('get'), + ], + }, + ], + roleVersion: defaultRoleVersion, + }, + ]; + const result = validateRoleEditorModel(model, undefined, undefined); + expect(validity(result.resources)).toEqual([false]); + }); + + test.each` + roleVersion | results + ${'v3'} | ${[false, true, false]} + ${'v4'} | ${[false, true, false]} + ${'v5'} | ${[false, true, false]} + ${'v6'} | ${[false, true, false]} + ${'v7'} | ${[true, true, true]} + `( + 'correct types of resources allowed for $roleVersion', + ({ roleVersion, results }) => { + const model = minimalRoleModel(); + model.resources = [ + { + kind: 'kube_cluster', + groups: [], + labels: [], + roleVersion, + resources: [ + { + ...newKubernetesResourceModel(defaultRoleVersion), + kind: kubernetesResourceKindOptionsMap.get('job'), + roleVersion, + }, + { + ...newKubernetesResourceModel(defaultRoleVersion), + kind: kubernetesResourceKindOptionsMap.get('pod'), + roleVersion, + }, + { + ...newKubernetesResourceModel(defaultRoleVersion), + kind: kubernetesResourceKindOptionsMap.get('service'), + roleVersion, + }, + ], + }, + ]; + const result = validateRoleEditorModel(model, undefined, undefined); + const resourceResult = result + .resources[0] as KubernetesAccessValidationResult; + expect(validity(resourceResult.fields.resources.results)).toEqual( + results + ); + } + ); + test('invalid access rule', () => { const model = minimalRoleModel(); model.rules = [ diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts index 051ca4988c1..a1292e06e5e 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts @@ -26,10 +26,14 @@ import { } from 'shared/components/Validation/rules'; import { nonEmptyLabels } from 'teleport/components/LabelsInput/LabelsInput'; -import { KubernetesResourceKind } from 'teleport/services/resources'; +import { + KubernetesResourceKind, + RoleVersion, +} from 'teleport/services/resources'; import { KubernetesResourceModel, + KubernetesVerbOption, MetadataModel, ResourceAccess, RoleEditorModel, @@ -171,6 +175,7 @@ export type ResourceAccessValidationResult = | WindowsDesktopAccessValidationResult; const validKubernetesResource = (res: KubernetesResourceModel) => () => { + const kind = validKubernetesKind(res.kind.value, res.roleVersion); const name = requiredField( 'Resource name is required, use "*" for any resource' )(res.name)(); @@ -179,15 +184,64 @@ const validKubernetesResource = (res: KubernetesResourceModel) => () => { : requiredField('Namespace is required for resources of this kind')( res.namespace )(); + const verbs = validKubernetesVerbs(res.verbs); return { - valid: name.valid && namespace.valid, + valid: kind.valid && name.valid && namespace.valid && verbs.valid, + kind, name, namespace, + verbs, }; }; export type KubernetesResourceValidationResult = { + kind: ValidationResult; name: ValidationResult; namespace: ValidationResult; + verbs: ValidationResult; +}; + +/** + * Validates a `kind` field of a `KubernetesResourceModel`. In roles with + * version v6, the auth server allows only specifying access for `pod` + * Kubernetes resources. + */ +const validKubernetesKind = ( + kind: KubernetesResourceKind, + ver: RoleVersion +): ValidationResult => { + switch (ver) { + case RoleVersion.V3: + case RoleVersion.V4: + case RoleVersion.V5: + case RoleVersion.V6: + const valid = kind === 'pod'; + return { + valid, + message: valid + ? undefined + : `Only pods are allowed for role version ${ver}`, + }; + + case RoleVersion.V7: + return { valid: true }; + + default: + ver satisfies never; + return { valid: true }; + } +}; + +const validKubernetesVerbs = ( + verbs: readonly KubernetesVerbOption[] +): ValidationResult => { + // Don't allow mixing '*' and other resource types. + const valid = verbs.length < 2 || verbs.every(v => v.value !== '*'); + return { + valid, + message: valid + ? undefined + : 'Mixing "All verbs" with other options is not allowed', + }; }; const kubernetesAccessValidationRules = { diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/withDefaults.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/withDefaults.ts index d99800812f6..93b7d9327f6 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/withDefaults.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/withDefaults.ts @@ -18,6 +18,8 @@ import { Role, RoleOptions } from 'teleport/services/resources'; +import { defaultRoleVersion } from './standardmodel'; + export type DeepPartial = { [k in keyof T]?: T[k] extends object ? DeepPartial : T[k]; }; @@ -31,7 +33,7 @@ export type DeepPartial = { */ export const withDefaults = (role: DeepPartial): Role => ({ kind: 'role', - version: '', + version: defaultRoleVersion, ...role, diff --git a/web/packages/teleport/src/services/resources/types.ts b/web/packages/teleport/src/services/resources/types.ts index cd763faa9e2..e5b42193388 100644 --- a/web/packages/teleport/src/services/resources/types.ts +++ b/web/packages/teleport/src/services/resources/types.ts @@ -46,6 +46,7 @@ export type RoleResource = Resource; */ export type Role = { kind: KindRole; + version: RoleVersion; metadata: { name: string; description?: string; @@ -58,9 +59,16 @@ export type Role = { deny: RoleConditions; options: RoleOptions; }; - version: string; }; +export enum RoleVersion { + V3 = 'v3', + V4 = 'v4', + V5 = 'v5', + V6 = 'v6', + V7 = 'v7', +} + /** * A set of conditions that must be matched to allow or deny access. Fields * follow the snake case convention to match the wire format. diff --git a/web/packages/teleterm/package.json b/web/packages/teleterm/package.json index 79529f225e4..a8b42a1495f 100644 --- a/web/packages/teleterm/package.json +++ b/web/packages/teleterm/package.json @@ -49,7 +49,6 @@ "electron-builder": "^25.1.8", "electron-vite": "^2.3.0", "events": "3.3.0", - "immer": "^10.1.1", "jest-canvas-mock": "^2.5.2", "react-dnd": "^14.0.4", "react-dnd-html5-backend": "^14.0.2",