Add admin rule tab to the role editor #49764
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
| 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 { | ||
| AccessSpecValidationResult, | ||
| AccessRuleValidationResult, | ||
| validateAccessSpec, | ||
| validateAccessRule, | ||
| } from './validation'; | ||
|
|
||
| const TestStandardEditor = (props: Partial<StandardEditorProps>) => { | ||
| const ctx = createTeleportContext(); | ||
|
|
@@ -58,13 +66,15 @@ const TestStandardEditor = (props: Partial<StandardEditorProps>) => { | |
| }); | ||
| return ( | ||
| <TeleportContextProvider ctx={ctx}> | ||
| <Validation> | ||
| <StandardEditor | ||
| originalRole={null} | ||
| standardEditorModel={model} | ||
| isProcessing={false} | ||
| onChange={setModel} | ||
| {...props} | ||
| /> | ||
| </Validation> | ||
| </TeleportContextProvider> | ||
| ); | ||
| }; | ||
|
|
@@ -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'); | ||
|
|
||
| function StatefulSection<Spec, ValidationResult>({ | ||
| defaultValue, | ||
| component: Component, | ||
| onChange, | ||
| validatorRef, | ||
| validate, | ||
| }: { | ||
| defaultValue: Spec; | ||
| component: React.ComponentType<SectionProps<Spec, any>>; | ||
| onChange(spec: Spec): void; | ||
| validatorRef?(v: Validator): void; | ||
| validate(arg: Spec): ValidationResult; | ||
| }) { | ||
| const [model, setModel] = useState<Spec>(defaultValue); | ||
| const validation = validate(model); | ||
| return ( | ||
| <Validation> | ||
| {({ validator }) => { | ||
|
|
@@ -196,20 +208,21 @@ const StatefulSection = <S extends AccessSpec>({ | |
| }} | ||
| </Validation> | ||
| ); | ||
| } | ||
|
|
||
| describe('ServerAccessSpecSection', () => { | ||
| const setup = () => { | ||
| const onChange = jest.fn(); | ||
| let validator: Validator; | ||
| render( | ||
| <StatefulSection<ServerAccessSpec, AccessSpecValidationResult> | ||
| component={ServerAccessSpecSection} | ||
| defaultValue={newAccessSpec('node')} | ||
|
There was a problem hiding this comment. Nit for the future, as it's not directly related to what's in this PR and changing this at this point could require a lot of not-strictly-necessary work: I noticed that those overloads on -export function newAccessSpec(kind: 'db'): DatabaseAccessSpec;
+export function newAccessSpec(kind: 'db'): AppAccessSpec;https://www.typescriptlang.org/docs/handbook/2/functions.html#writing-good-overloads
There's a couple of other solutions we could consider. If it's important for us to be able to call a function and get a specific variant of As a side note on overloads, TypeScript has a ton of features for expressing things that are legal in JavaScript, as TypeScript has to account for the fact that JavaScript is a dynamic language and that there are libraries which weren't written with JS in mind. But when authoring code, I found that usually I'm better off pretending that TypeScript is a static language with a type system akin to that found in Rust, F#, Haskell, or Elm. That is, I let go of the constructs like overloads which are supposed to help with typing JS code and make use mostly of product and sum types. There was a problem hiding this comment. Thanks, I'll keep it in mind. |
||
| 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( | ||
| <StatefulSection<KubernetesAccessSpec, AccessSpecValidationResult> | ||
| 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( | ||
| <StatefulSection<AppAccessSpec, AccessSpecValidationResult> | ||
| 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( | ||
| <StatefulSection<DatabaseAccessSpec, AccessSpecValidationResult> | ||
| 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( | ||
| <StatefulSection<WindowsDesktopAccessSpec, AccessSpecValidationResult> | ||
| 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( | ||
| <StatefulSection<RuleModel[], AccessRuleValidationResult[]> | ||
| 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()); | ||
|
There was a problem hiding this comment. Is there a way to trigger validation by interacting with the UI rather than depending on There was a problem hiding this comment. There would, of course, be a way if I wrapped the component with a button as a part of our test case. I just thought it's unnecessary, since the component under test in this case doesn't have that capability built-in, so it doesn't matter whether we add UI around it just for the testing purposes or not. Why do you think it would be beneficial? There was a problem hiding this comment. We often depend on some internal deps within our tests which then leads to brittle or flaky tests, see #49870. But I suppose using |
||
| 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'); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this comment block is almost guaranteed to be ignored, but I don't really have a better idea how to solve this. If I was ever adding a new kind, I'd do "Go to definition" on an existing, similar kind and never see this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the same, but that's the least I could do. The ultimatlely correct solution for this one would probably be consuming a protobuf enum, but that would be a much bigger undertaking.