From ea29404491d98d2265a9061ec42b8e38983d69da Mon Sep 17 00:00:00 2001 From: Bartosz Leper Date: Tue, 10 Dec 2024 10:51:12 +0100 Subject: [PATCH] Add admin rule tab to the role editor (#49764) (#49953) * Add admin rule tab to the role editor Also tightens some constraints about standard role editor conformance. * Add a way to delete an admin rule * Review --- api/types/constants.go | 5 + .../src/Roles/RoleEditor/RoleEditor.story.tsx | 2 +- .../src/Roles/RoleEditor/RoleEditor.tsx | 107 ++--- .../Roles/RoleEditor/StandardEditor.test.tsx | 118 +++++- .../src/Roles/RoleEditor/StandardEditor.tsx | 376 +++++++++++------- .../Roles/RoleEditor/standardmodel.test.ts | 331 ++++++++++++--- .../src/Roles/RoleEditor/standardmodel.ts | 179 +++++++-- .../src/Roles/RoleEditor/validation.ts | 14 + .../teleport/src/services/resources/types.ts | 184 +++++++++ 9 files changed, 1026 insertions(+), 290 deletions(-) diff --git a/api/types/constants.go b/api/types/constants.go index f1cf02f8bd888..369cc46730bf0 100644 --- a/api/types/constants.go +++ b/api/types/constants.go @@ -21,6 +21,11 @@ import ( ) const ( + // The `Kind*` constants in this const block identify resource kinds used for + // storage an/or and access control. Please keep these in sync with the + // `ResourceKind` enum in + // `web/packages/teleport/src/services/resources/types.ts`. + // DefaultAPIGroup is a default group of permissions API, // lets us to add different permission types DefaultAPIGroup = "gravitational.io/teleport" diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx index bf8313ceb4122..abd6ed5393f69 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.story.tsx @@ -42,7 +42,7 @@ export default { } return ( - + diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx index 38e2902ac2878..6b084a65682c4 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx @@ -20,6 +20,8 @@ import { Alert, Flex } from 'design'; import React, { useId, useState } from 'react'; import { useAsync } from 'shared/hooks/useAsync'; +import Validation, { Validator } from 'shared/components/Validation'; + import { Role, RoleWithYaml } from 'teleport/services/resources'; import { yamlService } from 'teleport/services/yaml'; import { YamlSupportedResourceKind } from 'teleport/services/yaml/types'; @@ -119,11 +121,18 @@ export const RoleEditor = ({ yamlifyAttempt.status === 'processing' || saveAttempt.status === 'processing'; - async function onTabChange(activeIndex: EditorTab) { + async function onTabChange(activeIndex: EditorTab, validator: Validator) { // The code below is not idempotent, so we need to protect ourselves from // an accidental model replacement. if (activeIndex === selectedEditorTab) return; + // Validate the model on tab switch, because the server-side yamlification + // requires model to be valid. However, if it's OK, we reset the validator. + // We don't want it to be validating at this point, since the user didn't + // attempt to submit the form. + if (!validator.validate()) return; + validator.reset(); + switch (activeIndex) { case EditorTab.Standard: { if (!yamlModel.content) { @@ -169,55 +178,59 @@ export const RoleEditor = ({ } return ( - - - {saveAttempt.status === 'error' && ( - - {saveAttempt.statusText} - - )} - {parseAttempt.status === 'error' && ( - - {parseAttempt.statusText} - - )} - {yamlifyAttempt.status === 'error' && ( - - {yamlifyAttempt.statusText} - - )} - {selectedEditorTab === EditorTab.Standard && ( -
- handleSave({ object })} - onCancel={handleCancel} - standardEditorModel={standardModel} - isProcessing={isProcessing} - onChange={setStandardModel} - /> -
- )} - {selectedEditorTab === EditorTab.Yaml && ( - - void (await handleSave({ yaml }))} + + {({ validator }) => ( + + onTabChange(index, validator)} isProcessing={isProcessing} - onCancel={handleCancel} - originalRole={originalRole} + standardEditorId={standardEditorId} + yamlEditorId={yamlEditorId} /> + {saveAttempt.status === 'error' && ( + + {saveAttempt.statusText} + + )} + {parseAttempt.status === 'error' && ( + + {parseAttempt.statusText} + + )} + {yamlifyAttempt.status === 'error' && ( + + {yamlifyAttempt.statusText} + + )} + {selectedEditorTab === EditorTab.Standard && ( +
+ handleSave({ object })} + onCancel={handleCancel} + standardEditorModel={standardModel} + isProcessing={isProcessing} + onChange={setStandardModel} + /> +
+ )} + {selectedEditorTab === EditorTab.Yaml && ( + + 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 59a7af9928081..fc8a1574637f0 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx @@ -26,19 +26,22 @@ import selectEvent from 'react-select-event'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; import { createTeleportContext } from 'teleport/mocks/contexts'; +import { ResourceKind } from 'teleport/services/resources'; + import { - AccessSpec, AppAccessSpec, DatabaseAccessSpec, KubernetesAccessSpec, newAccessSpec, newRole, roleToRoleEditorModel, + RuleModel, ServerAccessSpec, StandardEditorModel, WindowsDesktopAccessSpec, } from './standardmodel'; import { + AccessRules, AppAccessSpecSection, DatabaseAccessSpecSection, KubernetesAccessSpecSection, @@ -48,7 +51,12 @@ import { StandardEditorProps, WindowsDesktopAccessSpecSection, } from './StandardEditor'; -import { validateAccessSpec } from './validation'; +import { + AccessSpecValidationResult, + AccessRuleValidationResult, + validateAccessSpec, + validateAccessRule, +} from './validation'; const TestStandardEditor = (props: Partial) => { const ctx = createTeleportContext(); @@ -58,13 +66,15 @@ const TestStandardEditor = (props: Partial) => { }); return ( - + + + ); }; @@ -165,19 +175,21 @@ const getSectionByName = (name: string) => // eslint-disable-next-line testing-library/no-node-access screen.getByRole('heading', { level: 3, name }).closest('details'); -const StatefulSection = ({ +function StatefulSection({ defaultValue, component: Component, onChange, validatorRef, + validate, }: { - defaultValue: S; - component: React.ComponentType>; - onChange(spec: S): void; + defaultValue: Spec; + component: React.ComponentType>; + onChange(spec: Spec): void; validatorRef?(v: Validator): void; -}) => { - const [model, setModel] = useState(defaultValue); - const validation = validateAccessSpec(model); + validate(arg: Spec): ValidationResult; +}) { + const [model, setModel] = useState(defaultValue); + const validation = validate(model); return ( {({ validator }) => { @@ -196,20 +208,21 @@ const StatefulSection = ({ }} ); -}; +} describe('ServerAccessSpecSection', () => { const setup = () => { const onChange = jest.fn(); let validator: Validator; render( - + component={ServerAccessSpecSection} defaultValue={newAccessSpec('node')} onChange={onChange} validatorRef={v => { validator = v; }} + validate={validateAccessSpec} /> ); return { user: userEvent.setup(), onChange, validator }; @@ -258,13 +271,14 @@ describe('KubernetesAccessSpecSection', () => { const onChange = jest.fn(); let validator: Validator; render( - + component={KubernetesAccessSpecSection} defaultValue={newAccessSpec('kube_cluster')} onChange={onChange} validatorRef={v => { validator = v; }} + validate={validateAccessSpec} /> ); return { user: userEvent.setup(), onChange, validator }; @@ -399,13 +413,14 @@ describe('AppAccessSpecSection', () => { const onChange = jest.fn(); let validator: Validator; render( - + component={AppAccessSpecSection} defaultValue={newAccessSpec('app')} onChange={onChange} validatorRef={v => { validator = v; }} + validate={validateAccessSpec} /> ); return { user: userEvent.setup(), onChange, validator }; @@ -476,13 +491,14 @@ describe('DatabaseAccessSpecSection', () => { const onChange = jest.fn(); let validator: Validator; render( - + component={DatabaseAccessSpecSection} defaultValue={newAccessSpec('db')} onChange={onChange} validatorRef={v => { validator = v; }} + validate={validateAccessSpec} /> ); return { user: userEvent.setup(), onChange, validator }; @@ -532,13 +548,14 @@ describe('WindowsDesktopAccessSpecSection', () => { const onChange = jest.fn(); let validator: Validator; render( - + component={WindowsDesktopAccessSpecSection} defaultValue={newAccessSpec('windows_desktop')} onChange={onChange} validatorRef={v => { validator = v; }} + validate={validateAccessSpec} /> ); return { user: userEvent.setup(), onChange, validator }; @@ -569,6 +586,63 @@ describe('WindowsDesktopAccessSpecSection', () => { }); }); +describe('AccessRules', () => { + const setup = () => { + const onChange = jest.fn(); + let validator: Validator; + render( + + component={AccessRules} + defaultValue={[]} + onChange={onChange} + validatorRef={v => { + validator = v; + }} + validate={rules => rules.map(validateAccessRule)} + /> + ); + return { user: userEvent.setup(), onChange, validator }; + }; + + test('editing', async () => { + const { user, onChange } = setup(); + await user.click(screen.getByRole('button', { name: 'Add New' })); + await selectEvent.select(screen.getByLabelText('Resources'), [ + 'db', + 'node', + ]); + await selectEvent.select(screen.getByLabelText('Permissions'), [ + 'list', + 'read', + ]); + expect(onChange).toHaveBeenLastCalledWith([ + { + id: expect.any(String), + resources: [ + { label: ResourceKind.Database, value: 'db' }, + { label: ResourceKind.Node, value: 'node' }, + ], + verbs: [ + { label: 'list', value: 'list' }, + { label: 'read', value: 'read' }, + ], + }, + ] as RuleModel[]); + }); + + test('validation', async () => { + const { user, validator } = setup(); + await user.click(screen.getByRole('button', { name: 'Add New' })); + act(() => validator.validate()); + expect( + screen.getByText('At least one resource kind is required') + ).toBeInTheDocument(); + expect( + screen.getByText('At least one permission is required') + ).toBeInTheDocument(); + }); +}); + const reactSelectValueContainer = (input: HTMLInputElement) => // eslint-disable-next-line testing-library/no-node-access input.closest('.react-select__value-container'); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx index bf1567ee235cd..f17dce9ff179f 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx @@ -28,10 +28,7 @@ import { Text, } from 'design'; import FieldInput from 'shared/components/FieldInput'; -import Validation, { - useValidation, - Validator, -} from 'shared/components/Validation'; +import { useValidation } from 'shared/components/Validation'; import { precomputed, ValidationResult, @@ -69,6 +66,10 @@ import { AppAccessSpec, DatabaseAccessSpec, WindowsDesktopAccessSpec, + RuleModel, + resourceKindOptions, + verbOptions, + newRuleModel, } from './standardmodel'; import { validateRoleEditorModel, @@ -80,6 +81,7 @@ import { AppSpecValidationResult, DatabaseSpecValidationResult, WindowsDesktopSpecValidationResult, + AccessRuleValidationResult, } from './validation'; import { EditorSaveCancelButton } from './Shared'; import { RequiresResetToStandard } from './RequiresResetToStandard'; @@ -117,7 +119,7 @@ export const StandardEditor = ({ enum StandardEditorTab { Overview, Resources, - AdminRules, + AccessRules, Options, } @@ -125,10 +127,12 @@ export const StandardEditor = ({ const idPrefix = useId(); const overviewTabId = `${idPrefix}-overview`; const resourcesTabId = `${idPrefix}-resources`; - const adminRulesTabId = `${idPrefix}-admin-rules`; + const accessRulesTabId = `${idPrefix}-access-rules`; const optionsTabId = `${idPrefix}-options`; - function handleSave(validator: Validator) { + const validator = useValidation(); + + function handleSave() { if (!validator.validate()) { return; } @@ -188,146 +192,165 @@ export const StandardEditor = ({ }); } + function setRules(rules: RuleModel[]) { + handleChange({ + ...standardEditorModel.roleModel, + rules, + }); + } + return ( - - {({ validator }) => ( - <> - {roleModel.requiresReset && ( - - )} - - - !s.valid) - ? validationErrorTabStatus - : undefined, - }, - { - key: StandardEditorTab.AdminRules, - title: 'Admin Rules', - controls: adminRulesTabId, + <> + {roleModel.requiresReset && ( + + )} + + + !s.valid) + ? validationErrorTabStatus + : undefined, + }, + { + key: StandardEditorTab.AccessRules, + title: 'Access Rules', + controls: accessRulesTabId, + status: + validator.state.validating && + validation.rules.some(s => !s.valid) + ? validationErrorTabStatus + : undefined, + }, + { + key: StandardEditorTab.Options, + title: 'Options', + controls: optionsTabId, + }, + ]} + activeIndex={currentTab} + onChange={setCurrentTab} + /> + +
+ handleChange({ ...roleModel, metadata })} + /> +
+
+ + {roleModel.accessSpecs.map((spec, i) => { + const validationResult = validation.accessSpecs[i]; + return ( + setAccessSpec(value)} + onRemove={() => removeAccessSpec(spec.kind)} + /> + ); + })} + + + }} + buttonText={ + <> + + Add New Specifications + + } + buttonProps={{ + size: 'medium', + fill: 'filled', + disabled: isProcessing || allowedSpecKinds.length === 0, + }} + > + {allowedSpecKinds.map(kind => ( + addAccessSpec(kind)}> + {specSections[kind].title} + + ))} + -
- handleChange({ ...roleModel, metadata })} - /> -
-
- - {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)} - onCancel={onCancel} - disabled={ - isProcessing || - standardEditorModel.roleModel.requiresReset || - !standardEditorModel.isDirty - } - isEditing={isEditing} + +
+
+ - - )} - +
+ + handleSave()} + onCancel={onCancel} + disabled={ + isProcessing || + standardEditorModel.roleModel.requiresReset || + !standardEditorModel.isDirty + } + isEditing={isEditing} + /> + ); }; -export type SectionProps = { - value: T; +export type SectionProps = { + value: Model; isProcessing: boolean; - validation?: V; - onChange?(value: T): void; + validation?: ValidationResult; + onChange?(value: Model): void; }; const validationErrorTabStatus = { @@ -896,6 +919,83 @@ export function WindowsDesktopAccessSpecSection({ ); } +export function AccessRules({ + value, + isProcessing, + validation, + onChange, +}: SectionProps) { + 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)); + } + return ( + + {value.map((rule, i) => ( + removeRule(rule.id)} + /> + ))} + + + Add New + + + ); +} + +function AccessRule({ + value, + isProcessing, + validation, + onChange, + onRemove, +}: SectionProps & { + onRemove?(): void; +}) { + const { resources, verbs } = value; + return ( +
+ onChange?.({ ...value, resources: r })} + rule={precomputed(validation.fields.resources)} + /> + onChange?.({ ...value, verbs: v })} + rule={precomputed(validation.fields.verbs)} + mb={0} + /> +
+ ); +} + export const EditorWrapper = styled(Box)<{ mute?: boolean }>` opacity: ${p => (p.mute ? 0.4 : 1)}; pointer-events: ${p => (p.mute ? 'none' : '')}; diff --git a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts index 616e36673bbfe..157eee00c85e3 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.test.ts @@ -16,7 +16,12 @@ * along with this program. If not, see . */ -import { Role } from 'teleport/services/resources'; +import { + KubernetesResource, + ResourceKind, + Role, + Rule, +} from 'teleport/services/resources'; import { Label as UILabel } from 'teleport/components/LabelsInput/LabelsInput'; @@ -25,6 +30,7 @@ import { Labels } from 'teleport/services/resources'; import { labelsModelToLabels, labelsToModel, + newAccessSpec, RoleEditorModel, roleEditorModelToRole, roleToRoleEditorModel, @@ -37,6 +43,7 @@ const minimalRole = () => const minimalRoleModel = (): RoleEditorModel => ({ metadata: { name: 'foobar' }, accessSpecs: [], + rules: [], requiresReset: false, }); @@ -211,88 +218,217 @@ describe.each<{ name: string; role: Role; model: RoleEditorModel }>([ }); describe('roleToRoleEditorModel', () => { - it('detects unknown fields', () => { - const minRole = minimalRole(); - const roleModelWithReset: RoleEditorModel = { - ...minimalRoleModel(), - requiresReset: true, - }; - - expect(roleToRoleEditorModel(minRole).requiresReset).toEqual(false); - - expect( - roleToRoleEditorModel({ ...minRole, unknownField: 123 } as Role) - ).toEqual(roleModelWithReset); + const minRole = minimalRole(); + const roleModelWithReset: RoleEditorModel = { + ...minimalRoleModel(), + requiresReset: true, + }; + + test.each<{ name: string; role: Role; model?: RoleEditorModel }>([ + { + name: 'unknown fields in Role', + role: { ...minRole, unknownField: 123 } as Role, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in metadata', + role: { ...minRole, metadata: { name: 'foobar', unknownField: 123 }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in spec', + role: { ...minRole, spec: { ...minRole.spec, unknownField: 123 }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in spec.allow', + role: { ...minRole, spec: { ...minRole.spec, allow: { ...minRole.spec.allow, unknownField: 123 }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in KubernetesResource', + role: { ...minRole, spec: { ...minRole.spec, - deny: { ...minRole.spec.deny, unknownField: 123 }, + allow: { + ...minRole.spec.allow, + kubernetes_resources: [ + { kind: 'job', unknownField: 123 } as KubernetesResource, + ], + }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + model: { + ...roleModelWithReset, + accessSpecs: [ + { + ...newAccessSpec('kube_cluster'), + resources: [expect.any(Object)], + }, + ], + }, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unsupported resource kind in KubernetesResource', + role: { ...minRole, spec: { ...minRole.spec, - deny: { ...minRole.spec.deny, unknownField: 123 }, + allow: { + ...minRole.spec.allow, + kubernetes_resources: [ + { kind: 'illegal' } as unknown as KubernetesResource, + { kind: 'job' }, + ], + }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + model: { + ...roleModelWithReset, + accessSpecs: [ + { + ...newAccessSpec('kube_cluster'), + resources: [ + expect.objectContaining({ kind: { value: 'job', label: 'Job' } }), + ], + }, + ], + }, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unsupported verb in KubernetesResource', + role: { ...minRole, spec: { ...minRole.spec, - options: { ...minRole.spec.options, unknownField: 123 }, + allow: { + ...minRole.spec.allow, + kubernetes_resources: [ + { + kind: '*', + verbs: ['illegal', 'get'], + } as unknown as KubernetesResource, + ], + }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + model: { + ...roleModelWithReset, + accessSpecs: [ + { + ...newAccessSpec('kube_cluster'), + resources: [ + expect.objectContaining({ + verbs: [{ value: 'get', label: 'get' }], + }), + ], + }, + ], + }, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in Rule', + role: { ...minRole, spec: { ...minRole.spec, - options: { - ...minRole.spec.options, - idp: { saml: { enabled: true }, unknownField: 123 }, + allow: { + ...minRole.spec.allow, + rules: [{ unknownField: 123 } as Rule], }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + model: { + ...roleModelWithReset, + rules: [expect.any(Object)], + }, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unsupported resource kind in Rule', + role: { + ...minRole, + spec: { + ...minRole.spec, + allow: { + ...minRole.spec.allow, + rules: [{ resources: ['illegal', 'node'] } as unknown as Rule], + }, + }, + } as Role, + model: { + ...roleModelWithReset, + rules: [ + expect.objectContaining({ + resources: [{ value: 'node', label: 'node' }], + }), + ], + }, + }, + + { + name: 'unsupported verb in Rule', + role: { + ...minRole, + spec: { + ...minRole.spec, + allow: { + ...minRole.spec.allow, + rules: [{ verbs: ['illegal', 'create'] } as unknown as Rule], + }, + }, + } as Role, + model: { + ...roleModelWithReset, + rules: [ + expect.objectContaining({ + verbs: [{ value: 'create', label: 'create' }], + }), + ], + }, + }, + + { + name: 'unknown fields in spec.deny', + role: { + ...minRole, + spec: { + ...minRole.spec, + deny: { ...minRole.spec.deny, unknownField: 123 }, + }, + } as Role, + }, + + { + name: 'unknown fields in spec.options', + role: { + ...minRole, + spec: { + ...minRole.spec, + options: { ...minRole.spec.options, unknownField: 123 }, + }, + } as Role, + }, + + { + name: 'unknown fields in spec.options.idp.saml', + role: { ...minRole, spec: { ...minRole.spec, @@ -301,11 +437,12 @@ describe('roleToRoleEditorModel', () => { idp: { saml: { enabled: true, unknownField: 123 } }, }, }, - } as Role) - ).toEqual(roleModelWithReset); + } as Role, + }, - expect( - roleToRoleEditorModel({ + { + name: 'unknown fields in spec.options.record_session', + role: { ...minRole, spec: { ...minRole.spec, @@ -317,9 +454,14 @@ describe('roleToRoleEditorModel', () => { }, }, }, - } as Role) - ).toEqual(roleModelWithReset); - }); + } as Role, + }, + ])( + 'requires reset because of $name', + ({ role, model = roleModelWithReset }) => { + expect(roleToRoleEditorModel(role)).toEqual(model); + } + ); test('version change requires reset', () => { expect(roleToRoleEditorModel({ ...minimalRole(), version: 'v1' })).toEqual({ @@ -471,6 +613,46 @@ describe('roleToRoleEditorModel', () => { }); }); +it('creates a rule model', () => { + expect( + roleToRoleEditorModel({ + ...minimalRole(), + spec: { + ...minimalRole().spec, + allow: { + rules: [ + { + resources: [ResourceKind.User, ResourceKind.DatabaseService], + verbs: ['read', 'list'], + }, + { resources: [ResourceKind.Lock], verbs: ['create'] }, + ], + }, + }, + }) + ).toEqual({ + ...minimalRoleModel(), + rules: [ + { + id: expect.any(String), + resources: [ + { label: 'user', value: 'user' }, + { label: 'db_service', value: 'db_service' }, + ], + verbs: [ + { label: 'read', value: 'read' }, + { label: 'list', value: 'list' }, + ], + }, + { + id: expect.any(String), + resources: [{ label: 'lock', value: 'lock' }], + verbs: [{ label: 'create', value: 'create' }], + }, + ], + } as RoleEditorModel); +}); + test('labelsToModel', () => { expect(labelsToModel({ foo: 'bar', doubleFoo: ['bar1', 'bar2'] })).toEqual([ { name: 'foo', value: 'bar' }, @@ -562,6 +744,43 @@ describe('roleEditorModelToRole', () => { }, } as Role); }); + + it('converts a rule model', () => { + expect( + roleEditorModelToRole({ + ...minimalRoleModel(), + rules: [ + { + id: 'dummy-id-1', + resources: [ + { label: 'user', value: ResourceKind.User }, + { label: 'db_service', value: ResourceKind.DatabaseService }, + ], + verbs: [ + { label: 'read', value: 'read' }, + { label: 'list', value: 'list' }, + ], + }, + { + id: 'dummy-id-2', + resources: [{ label: 'lock', value: ResourceKind.Lock }], + verbs: [{ label: 'create', value: 'create' }], + }, + ], + }) + ).toEqual({ + ...minimalRole(), + spec: { + ...minimalRole().spec, + allow: { + rules: [ + { resources: ['user', 'db_service'], verbs: ['read', 'list'] }, + { resources: ['lock'], verbs: ['create'] }, + ], + }, + }, + } as Role); + }); }); test('labelsModelToLabels', () => { diff --git a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts index f50158add2ba6..6826957c127c2 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts @@ -29,6 +29,9 @@ import { Label as UILabel } from 'teleport/components/LabelsInput/LabelsInput'; import { KubernetesResourceKind, KubernetesVerb, + ResourceKind, + Rule, + Verb, } from 'teleport/services/resources/types'; import { defaultOptions } from './withDefaults'; @@ -49,6 +52,7 @@ export type StandardEditorModel = { export type RoleEditorModel = { metadata: MetadataModel; accessSpecs: AccessSpec[]; + rules: RuleModel[]; /** * Indicates whether the current resource, as described by YAML, is * accurately represented by this editor model. If it's not, the user needs @@ -150,6 +154,13 @@ export const kubernetesResourceKindOptions: KubernetesResourceKindOption[] = [ ).toSorted((a, b) => a.label.localeCompare(b.label)), ]; +const optionsToMap = (opts: Option[]) => + new Map(opts.map(o => [o.value, o])); + +const kubernetesResourceKindOptionsMap = optionsToMap( + kubernetesResourceKindOptions +); + type KubernetesVerbOption = Option; /** * All possible Kubernetes verb drop-down options. This array needs to be kept @@ -180,6 +191,33 @@ export const kubernetesVerbOptions: KubernetesVerbOption[] = [ .toSorted((a, b) => a.localeCompare(b)) .map(stringToOption), ]; +const kubernetesVerbOptionsMap = optionsToMap(kubernetesVerbOptions); + +type ResourceKindOption = Option; +export const resourceKindOptions: ResourceKindOption[] = Object.values( + ResourceKind +) + .toSorted() + .map(stringToOption); +const resourceKindOptionsMap = optionsToMap(resourceKindOptions); + +type VerbOption = Option; +export const verbOptions: VerbOption[] = ( + [ + '*', + 'create', + 'create_enroll_token', + 'delete', + 'enroll', + 'list', + 'read', + 'readnosecrets', + 'rotate', + 'update', + 'use', + ] as const +).map(stringToOption); +const verbOptionsMap = optionsToMap(verbOptions); /** Model for the server access specification section. */ export type ServerAccessSpec = AccessSpecBase<'node'> & { @@ -206,6 +244,13 @@ export type WindowsDesktopAccessSpec = AccessSpecBase<'windows_desktop'> & { logins: readonly Option[]; }; +export type RuleModel = { + /** Autogenerated ID to be used with the `key` property. */ + id: string; + resources: readonly ResourceKindOption[]; + verbs: readonly VerbOption[]; +}; + const roleVersion = 'v7'; /** @@ -267,6 +312,14 @@ export function newKubernetesResourceModel(): KubernetesResourceModel { }; } +export function newRuleModel(): RuleModel { + return { + id: crypto.randomUUID(), + resources: [], + verbs: [], + }; +} + /** * Converts a role to its in-editor UI model representation. The resulting * model may be marked as requiring reset if the role contains unsupported @@ -280,11 +333,14 @@ export function roleToRoleEditorModel( // has been left. Therefore, we don't want Lint to warn us that we didn't use // some of the fields. // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { kind, metadata, spec, version, ...rest } = role; - const { name, description, revision, ...mRest } = metadata; - const { allow, deny, options, ...sRest } = spec; - const { accessSpecs, requiresReset: allowRequiresReset } = - roleConditionsToAccessSpecs(allow); + const { kind, metadata, spec, version, ...unsupported } = role; + const { name, description, revision, ...unsupportedMetadata } = metadata; + const { allow, deny, options, ...unsupportedSpecs } = spec; + const { + accessSpecs, + rules, + requiresReset: allowRequiresReset, + } = roleConditionsToModel(allow); return { metadata: { @@ -293,13 +349,14 @@ export function roleToRoleEditorModel( revision: originalRole?.metadata?.revision, }, accessSpecs, + rules, requiresReset: revision !== originalRole?.metadata?.revision || version !== roleVersion || !( - isEmpty(rest) && - isEmpty(mRest) && - isEmpty(sRest) && + isEmpty(unsupported) && + isEmpty(unsupportedMetadata) && + isEmpty(unsupportedSpecs) && isEmpty(deny) && equalsDeep(options, defaultOptions()) ) || @@ -311,10 +368,9 @@ export function roleToRoleEditorModel( * Converts a `RoleConditions` instance (an "allow" or "deny" section, to be * specific) to a list of access specification models. */ -function roleConditionsToAccessSpecs(conditions: RoleConditions): { - accessSpecs: AccessSpec[]; - requiresReset: boolean; -} { +function roleConditionsToModel( + conditions: RoleConditions +): Pick { const { node_labels, logins, @@ -335,7 +391,10 @@ function roleConditionsToAccessSpecs(conditions: RoleConditions): { windows_desktop_labels, windows_desktop_logins, - ...rest + + rules, + + ...unsupportedConditions } = conditions; const accessSpecs: AccessSpec[] = []; @@ -352,7 +411,10 @@ function roleConditionsToAccessSpecs(conditions: RoleConditions): { const kubeGroupsModel = stringsToOptions(kubernetes_groups ?? []); const kubeLabelsModel = labelsToModel(kubernetes_labels); - const kubeResourcesModel = kubernetesResourcesToModel(kubernetes_resources); + const { + model: kubeResourcesModel, + requiresReset: kubernetesResourcesRequireReset, + } = kubernetesResourcesToModel(kubernetes_resources); if (someNonEmpty(kubeGroupsModel, kubeLabelsModel, kubeResourcesModel)) { accessSpecs.push({ kind: 'kube_cluster', @@ -409,9 +471,16 @@ function roleConditionsToAccessSpecs(conditions: RoleConditions): { }); } + const { model: rulesModel, requiresReset: rulesRequireReset } = + rulesToModel(rules); + return { accessSpecs, - requiresReset: !isEmpty(rest), + rules: rulesModel, + requiresReset: + kubernetesResourcesRequireReset || + rulesRequireReset || + !isEmpty(unsupportedConditions), }; } @@ -447,18 +516,69 @@ function stringsToOptions(arr: T[]): Option[] { function kubernetesResourcesToModel( resources: KubernetesResource[] | undefined -): KubernetesResourceModel[] { - return (resources ?? []).map( - ({ kind, name, namespace = '', verbs = [] }) => ({ +): { model: KubernetesResourceModel[]; requiresReset: boolean } { + const result = (resources ?? []).map(kubernetesResourceToModel); + return { + model: result.map(r => r.model).filter(m => m !== undefined), + requiresReset: result.some(r => r.requiresReset), + }; +} + +function kubernetesResourceToModel(res: KubernetesResource): { + model?: KubernetesResourceModel; + requiresReset: boolean; +} { + const { kind, name, namespace = '', verbs = [], ...unsupported } = res; + const kindOption = kubernetesResourceKindOptionsMap.get(kind); + const verbOptions = verbs.map(verb => kubernetesVerbOptionsMap.get(verb)); + const knownVerbOptions = verbOptions.filter(v => v !== undefined); + return { + model: + kindOption !== undefined + ? { + id: crypto.randomUUID(), + kind: kindOption, + name, + namespace, + verbs: knownVerbOptions, + } + : undefined, + requiresReset: + kindOption === undefined || + verbOptions.length !== knownVerbOptions.length || + !isEmpty(unsupported), + }; +} + +function rulesToModel(rules: Rule[]): { + model: RuleModel[]; + requiresReset: boolean; +} { + const result = (rules ?? []).map(ruleToModel); + return { + model: result.map(r => r.model), + requiresReset: result.some(r => r.requiresReset), + }; +} + +function ruleToModel(rule: Rule): { model: RuleModel; requiresReset: boolean } { + const { resources = [], verbs = [], ...unsupported } = rule; + const resourcesModel = resources.map(k => resourceKindOptionsMap.get(k)); + const knownResourcesModel = resourcesModel.filter(m => m !== undefined); + const verbsModel = verbs.map(v => verbOptionsMap.get(v)); + const knownVerbsModel = verbsModel.filter(m => m !== undefined); + const requiresReset = + !isEmpty(unsupported) || + knownResourcesModel.length !== resourcesModel.length || + knownVerbsModel.length !== verbs.length; + return { + model: { id: crypto.randomUUID(), - kind: kubernetesResourceKindOptions.find(o => o.value === kind), - name, - namespace, - verbs: verbs.map(verb => - kubernetesVerbOptions.find(o => o.value === verb) - ), - }) - ); + resources: knownResourcesModel, + verbs: knownVerbsModel, + }, + requiresReset, + }; } function isEmpty(obj: object) { @@ -535,6 +655,13 @@ export function roleEditorModelToRole(roleModel: RoleEditorModel): Role { } } + if (roleModel.rules.length > 0) { + role.spec.allow.rules = roleModel.rules.map(role => ({ + resources: role.resources.map(r => r.value), + verbs: role.verbs.map(v => v.value), + })); + } + return role; } diff --git a/web/packages/teleport/src/Roles/RoleEditor/validation.ts b/web/packages/teleport/src/Roles/RoleEditor/validation.ts index 95cde89ed036c..d7ca5b05f7144 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/validation.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/validation.ts @@ -35,6 +35,7 @@ import { KubernetesResourceModel, MetadataModel, RoleEditorModel, + RuleModel, } from './standardmodel'; const kubernetesClusterWideResourceKinds: KubernetesResourceKind[] = [ @@ -49,10 +50,12 @@ const kubernetesClusterWideResourceKinds: KubernetesResourceKind[] = [ export function validateRoleEditorModel({ metadata, accessSpecs, + rules, }: RoleEditorModel) { return { metadata: validateMetadata(metadata), accessSpecs: accessSpecs.map(validateAccessSpec), + rules: rules.map(validateAccessRule), }; } @@ -166,3 +169,14 @@ const windowsDesktopSpecValidationRules = { export type WindowsDesktopSpecValidationResult = RuleSetValidationResult< typeof windowsDesktopSpecValidationRules >; + +export const validateAccessRule = (accessRule: RuleModel) => + runRules(accessRule, accessRuleValidationRules); + +const accessRuleValidationRules = { + resources: requiredField('At least one resource kind is required'), + verbs: requiredField('At least one permission is required'), +}; +export type AccessRuleValidationResult = RuleSetValidationResult< + typeof accessRuleValidationRules +>; diff --git a/web/packages/teleport/src/services/resources/types.ts b/web/packages/teleport/src/services/resources/types.ts index c5ab066e2c894..c5a17f313e872 100644 --- a/web/packages/teleport/src/services/resources/types.ts +++ b/web/packages/teleport/src/services/resources/types.ts @@ -83,6 +83,8 @@ export type RoleConditions = { windows_desktop_labels?: Labels; windows_desktop_logins?: string[]; + + rules?: Rule[]; }; export type Labels = Record; @@ -143,6 +145,188 @@ export type KubernetesVerb = | 'exec' | 'portforward'; +export type Rule = { + resources?: ResourceKind[]; + verbs?: Verb[]; +}; + +export enum ResourceKind { + Wildcard = '*', + + // This list was taken from all of the `Kind*` constants in + // `api/types/constants.go`. Please keep these in sync. + + // Resources backed by objects in the backend database. + AccessGraphSecretAuthorizedKey = 'access_graph_authorized_key', + AccessGraphSecretPrivateKey = 'access_graph_private_key', + AccessGraphSettings = 'access_graph_settings', + AccessList = 'access_list', + AccessListMember = 'access_list_member', + AccessListReview = 'access_list_review', + AccessMonitoringRule = 'access_monitoring_rule', + AccessRequest = 'access_request', + App = 'app', + AppOrSAMLIdPServiceProvider = 'app_server_or_saml_idp_sp', + AppServer = 'app_server', + AuditQuery = 'audit_query', + AuthServer = 'auth_server', + AutoUpdateAgentRollout = 'autoupdate_agent_rollout', + AutoUpdateConfig = 'autoupdate_config', + AutoUpdateVersion = 'autoupdate_version', + Bot = 'bot', + BotInstance = 'bot_instance', + CertAuthority = 'cert_authority', + ClusterAlert = 'cluster_alert', + ClusterAuditConfig = 'cluster_audit_config', + ClusterAuthPreference = 'cluster_auth_preference', + ClusterMaintenanceConfig = 'cluster_maintenance_config', + ClusterName = 'cluster_name', + ClusterNetworkingConfig = 'cluster_networking_config', + ConnectionDiagnostic = 'connection_diagnostic', + CrownJewel = 'crown_jewel', + Database = 'db', + DatabaseObject = 'db_object', + DatabaseObjectImportRule = 'db_object_import_rule', + DatabaseServer = 'db_server', + DatabaseService = 'db_service', + Device = 'device', + DiscoveryConfig = 'discovery_config', + DynamicWindowsDesktop = 'dynamic_windows_desktop', + ExternalAuditStorage = 'external_audit_storage', + GitServer = 'git_server', + // Ignoring duplicate: KindGithub = "github" + GithubConnector = 'github', + GlobalNotification = 'global_notification', + HeadlessAuthentication = 'headless_authentication', + Identity = 'identity', + IdentityCenterAccount = 'aws_ic_account', + IdentityCenterAccountAssignment = 'aws_ic_account_assignment', + IdentityCenterPermissionSet = 'aws_ic_permission_set', + IdentityCenterPrincipalAssignment = 'aws_ic_principal_assignment', + Installer = 'installer', + Instance = 'instance', + Integration = 'integration', + KubeCertificateSigningRequest = 'certificatesigningrequest', + KubeClusterRole = 'clusterrole', + KubeClusterRoleBinding = 'clusterrolebinding', + KubeConfigmap = 'configmap', + KubeCronjob = 'cronjob', + KubeDaemonSet = 'daemonset', + KubeDeployment = 'deployment', + KubeIngress = 'ingress', + KubeJob = 'job', + KubeNamespace = 'namespace', + KubeNode = 'kube_node', + KubePersistentVolume = 'persistentvolume', + KubePersistentVolumeClaim = 'persistentvolumeclaim', + KubePod = 'pod', + KubeReplicaSet = 'replicaset', + KubeRole = 'kube_role', + KubeRoleBinding = 'rolebinding', + KubeSecret = 'secret', + KubeServer = 'kube_server', + KubeService = 'service', + KubeServiceAccount = 'serviceaccount', + KubeStatefulset = 'statefulset', + KubeWaitingContainer = 'kube_ephemeral_container', + KubernetesCluster = 'kube_cluster', + License = 'license', + Lock = 'lock', + LoginRule = 'login_rule', + MFADevice = 'mfa_device', + // Ignoring duplicate: KindNamespace = "namespace" + NetworkRestrictions = 'network_restrictions', + Node = 'node', + Notification = 'notification', + // Ignoring duplicate: KindOIDC = "oidc" + OIDCConnector = 'oidc', + OktaAssignment = 'okta_assignment', + OktaImportRule = 'okta_import_rule', + Plugin = 'plugin', + PluginData = 'plugin_data', + PluginStaticCredentials = 'plugin_static_credentials', + ProvisioningPrincipalState = 'provisioning_principal_state', + Proxy = 'proxy', + RecoveryCodes = 'recovery_codes', + RemoteCluster = 'remote_cluster', + ReverseTunnel = 'tunnel', + Role = 'role', + // Ignoring duplicate: KindSAML = "saml" + SAMLConnector = 'saml', + SAMLIdPServiceProvider = 'saml_idp_service_provider', + SPIFFEFederation = 'spiffe_federation', + SecurityReport = 'security_report', + SecurityReportCostLimiter = 'security_report_cost_limiter', + SecurityReportState = 'security_report_state', + Semaphore = 'semaphore', + ServerInfo = 'server_info', + SessionRecordingConfig = 'session_recording_config', + SessionTracker = 'session_tracker', + State = 'state', + StaticHostUser = 'static_host_user', + StaticTokens = 'static_tokens', + Token = 'token', + TrustedCluster = 'trusted_cluster', + TunnelConnection = 'tunnel_connection', + UIConfig = 'ui_config', + User = 'user', + UserGroup = 'user_group', + UserLastSeenNotification = 'user_last_seen_notification', + UserLoginState = 'user_login_state', + UserNotificationState = 'user_notification_state', + UserTask = 'user_task', + UserToken = 'user_token', + UserTokenSecrets = 'user_token_secrets', + VnetConfig = 'vnet_config', + WatchStatus = 'watch_status', + WebSession = 'web_session', + WebToken = 'web_token', + WindowsDesktop = 'windows_desktop', + WindowsDesktopService = 'windows_desktop_service', + + // Resources that have no actual data representation, but serve for checking + // access to various features. + AccessGraph = 'access_graph', + AccessPluginData = 'access_plugin_data', + AuthConnector = 'auth_connector', + Billing = 'billing', + ClusterConfig = 'cluster_config', + Connectors = 'connectors', + DatabaseCertificate = 'database_certificate', + Download = 'download', + Event = 'event', + GithubRequest = 'github_request', + HostCert = 'host_cert', + IdentityCenter = 'aws_identity_center', + JWT = 'jwt', + OIDCRequest = 'oidc_request', + SAMLRequest = 'saml_request', + SSHSession = 'ssh_session', + Session = 'session', + UnifiedResource = 'unified_resource', + UsageEvent = 'usage_event', + + // For completeness: these kind constants were not included here, as they + // refer to resource subkind names that are not used for access control. + // + // KindAppSession = "app_session" + // KindSAMLIdPSession = "saml_idp_session" + // KindSnowflakeSession = "snowflake_session" +} + +export type Verb = + | '*' + | 'create' + | 'create_enroll_token' + | 'delete' + | 'enroll' + | 'list' + | 'read' + | 'readnosecrets' + | 'rotate' + | 'update' + | 'use'; + /** * 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.