Skip to content

Commit

Permalink
More tweaks to the role editor UI (#50174)
Browse files Browse the repository at this point in the history
* More tweaks to the role editor UI

- Add default trait references to resource access definitions
- Remove the role deletion button

* Type check fix
  • Loading branch information
bl-nero authored Dec 13, 2024
1 parent 26f2187 commit 5ebc5bf
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 92 deletions.
29 changes: 1 addition & 28 deletions web/packages/teleport/src/Roles/RoleEditor/EditorHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,15 @@

import React from 'react';
import { Flex, H2, Indicator, Box, ButtonIcon } from 'design';
import { HoverTooltip } from 'design/Tooltip';
import { Cross } from 'design/Icon';

import { Cross, Trash } from 'design/Icon';

import useTeleport from 'teleport/useTeleport';
import { Role } from 'teleport/services/resources';

import { EditorTab, EditorTabs } from './EditorTabs';

/** Renders a header button with role name and delete button. */
export const EditorHeader = ({
role = null,
onDelete,
selectedEditorTab,
onEditorTabChange,
isProcessing,
Expand All @@ -39,19 +35,15 @@ export const EditorHeader = ({
onClose,
}: {
role?: Role;
onDelete(): void;
selectedEditorTab: EditorTab;
onEditorTabChange(t: EditorTab): void;
isProcessing: boolean;
standardEditorId: string;
yamlEditorId: string;
onClose(): void;
}) => {
const ctx = useTeleport();
const isCreating = !role;

const hasDeleteAccess = ctx.storeUser.getRoleAccess().remove;

return (
<Flex alignItems="center" mb={3} gap={2}>
<ButtonIcon aria-label="Close" onClick={onClose}>
Expand All @@ -74,25 +66,6 @@ export const EditorHeader = ({
standardEditorId={standardEditorId}
yamlEditorId={yamlEditorId}
/>
{!isCreating && (
<HoverTooltip
position="bottom"
tipContent={
hasDeleteAccess
? 'Delete'
: 'You do not have access to delete a role'
}
>
<ButtonIcon
onClick={onDelete}
disabled={!hasDeleteAccess}
data-testid="delete"
p={1}
>
<Trash size="medium" />
</ButtonIcon>
</HoverTooltip>
)}
</Flex>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,6 @@ export const Dialog: StoryObj = {
open={open}
onClose={() => setOpen(false)}
onSave={async () => setOpen(false)}
onDelete={async () => setOpen(false)}
/>
</>
);
Expand All @@ -311,7 +310,6 @@ export const DialogWithPolicyEnabled: StoryObj = {
open={open}
onClose={() => setOpen(false)}
onSave={async () => setOpen(false)}
onDelete={async () => setOpen(false)}
/>
</>
);
Expand Down
14 changes: 0 additions & 14 deletions web/packages/teleport/src/Roles/RoleEditor/RoleEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import { yamlService } from 'teleport/services/yaml';
import { YamlSupportedResourceKind } from 'teleport/services/yaml/types';
import { CaptureEvent, userEventService } from 'teleport/services/userEvent';

import DeleteRole from '../DeleteRole';

import {
roleEditorModelToRole,
newRole,
Expand All @@ -49,7 +47,6 @@ export type RoleEditorProps = {
originalRole?: RoleWithYaml;
onCancel?(): void;
onSave?(r: Partial<RoleWithYaml>): Promise<void>;
onDelete?(): Promise<void>;
};

/**
Expand All @@ -61,7 +58,6 @@ export const RoleEditor = ({
originalRole,
onCancel,
onSave,
onDelete,
}: RoleEditorProps) => {
const idPrefix = useId();
// These IDs are needed to connect accessibility attributes between the
Expand Down Expand Up @@ -90,8 +86,6 @@ export const RoleEditor = ({
standardModel.roleModel.requiresReset ? EditorTab.Yaml : EditorTab.Standard
);

const [deleting, setDeleting] = useState(false);

// Converts YAML representation to a standard editor model.
const [parseAttempt, parseYaml] = useAsync(async () => {
const parsedRole = await yamlService.parse<Role>(
Expand Down Expand Up @@ -188,7 +182,6 @@ export const RoleEditor = ({
<Box mt={3} mx={3}>
<EditorHeader
role={originalRole?.object}
onDelete={() => setDeleting(true)}
selectedEditorTab={selectedEditorTab}
onEditorTabChange={index => onTabChange(index, validator)}
isProcessing={isProcessing}
Expand Down Expand Up @@ -236,13 +229,6 @@ export const RoleEditor = ({
/>
</Flex>
)}
{deleting && (
<DeleteRole
name={originalRole.object.metadata.name}
onClose={() => setDeleting(false)}
onDelete={onDelete}
/>
)}
</Flex>
)}
</Validation>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,10 @@ import tagpromo from './tagpromo.png';
export function RoleEditorAdapter({
resources,
onSave,
onDelete,
onCancel,
}: {
resources: ResourcesState;
onSave: (role: Partial<RoleWithYaml>) => Promise<void>;
onDelete: () => Promise<void>;
onCancel: () => void;
}) {
const theme = useTheme();
Expand Down Expand Up @@ -106,7 +104,6 @@ export function RoleEditorAdapter({
originalRole={convertAttempt.data}
onCancel={onCancel}
onSave={onSave}
onDelete={onDelete}
/>
)}
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,11 @@ export function RoleEditorDialog({
onClose,
resources,
onSave,
onDelete,
}: {
open: boolean;
onClose(): void;
resources: ResourcesState;
onSave(role: Partial<RoleWithYaml>): Promise<void>;
onDelete(): Promise<void>;
}) {
const transitionRef = useRef<HTMLDivElement>();
return (
Expand All @@ -61,7 +59,6 @@ export function RoleEditorDialog({
transitionState={transitionState}
resources={resources}
onSave={onSave}
onDelete={onDelete}
/>
)}
</Transition>
Expand All @@ -75,9 +72,8 @@ const DialogInternal = forwardRef<
transitionState: TransitionStatus;
resources: ResourcesState;
onSave(role: Partial<RoleWithYaml>): Promise<void>;
onDelete(): Promise<void>;
}
>(({ onClose, transitionState, resources, onSave, onDelete }, ref) => {
>(({ onClose, transitionState, resources, onSave }, ref) => {
return (
<Dialog
dialogCss={() => fullScreenDialogCss()}
Expand All @@ -89,7 +85,6 @@ const DialogInternal = forwardRef<
<RoleEditorAdapter
resources={resources}
onSave={onSave}
onDelete={onDelete}
onCancel={onClose}
/>
</Dialog>
Expand Down
99 changes: 72 additions & 27 deletions web/packages/teleport/src/Roles/RoleEditor/StandardEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ describe('ServerAccessSpecSection', () => {
kind: 'node',
labels: [{ name: 'some-key', value: 'some-value' }],
logins: [
expect.objectContaining({
label: '{{internal.logins}}',
value: '{{internal.logins}}',
}),
expect.objectContaining({ label: 'root', value: 'root' }),
expect.objectContaining({ label: 'some-user', value: 'some-user' }),
],
Expand Down Expand Up @@ -317,6 +321,7 @@ describe('KubernetesAccessSpecSection', () => {
expect(onChange).toHaveBeenLastCalledWith({
kind: 'kube_cluster',
groups: [
expect.objectContaining({ value: '{{internal.kubernetes_groups}}' }),
expect.objectContaining({ value: 'group1' }),
expect.objectContaining({ value: 'group2' }),
],
Expand Down Expand Up @@ -426,61 +431,89 @@ describe('AppAccessSpecSection', () => {
return { user: userEvent.setup(), onChange, validator };
};

const awsRoleArn = () =>
within(screen.getByRole('group', { name: 'AWS Role ARNs' })).getByRole(
'textbox'
);
const azureIdentity = () =>
within(screen.getByRole('group', { name: 'Azure Identities' })).getByRole(
'textbox'
);
const gcpServiceAccount = () =>
within(
screen.getByRole('group', { name: 'GCP Service Accounts' })
).getByRole('textbox');
const awsRoleArns = () =>
screen.getByRole('group', { name: 'AWS Role ARNs' });
const awsRoleArnTextBoxes = () =>
within(awsRoleArns()).getAllByRole('textbox');
const azureIdentities = () =>
screen.getByRole('group', { name: 'Azure Identities' });
const azureIdentityTextBoxes = () =>
within(azureIdentities()).getAllByRole('textbox');
const gcpServiceAccounts = () =>
screen.getByRole('group', { name: 'GCP Service Accounts' });
const gcpServiceAccountTextBoxes = () =>
within(gcpServiceAccounts()).getAllByRole('textbox');

test('editing', async () => {
const { user, onChange } = setup();
await user.click(screen.getByRole('button', { name: 'Add a Label' }));
await user.type(screen.getByPlaceholderText('label key'), 'env');
await user.type(screen.getByPlaceholderText('label value'), 'prod');
await user.type(awsRoleArn(), 'arn:aws:iam::123456789012:role/admin');
await user.click(
within(awsRoleArns()).getByRole('button', { name: 'Add More' })
);
await user.type(
awsRoleArnTextBoxes()[1],
'arn:aws:iam::123456789012:role/admin'
);
await user.click(
within(azureIdentities()).getByRole('button', { name: 'Add More' })
);
await user.type(
azureIdentity(),
azureIdentityTextBoxes()[1],
'/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin'
);
await user.click(
within(gcpServiceAccounts()).getByRole('button', { name: 'Add More' })
);
await user.type(
gcpServiceAccount(),
gcpServiceAccountTextBoxes()[1],
'[email protected]'
);
expect(onChange).toHaveBeenLastCalledWith({
kind: 'app',
labels: [{ name: 'env', value: 'prod' }],
awsRoleARNs: ['arn:aws:iam::123456789012:role/admin'],
awsRoleARNs: [
'{{internal.aws_role_arns}}',
'arn:aws:iam::123456789012:role/admin',
],
azureIdentities: [
'{{internal.azure_identities}}',
'/subscriptions/1020304050607-cafe-8090-a0b0c0d0e0f0/resourceGroups/example-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/admin',
],
gcpServiceAccounts: ['[email protected]'],
gcpServiceAccounts: [
'{{internal.gcp_service_accounts}}',
'[email protected]',
],
} as AppAccessSpec);
});

test('validation', async () => {
const { user, validator } = setup();
await user.click(screen.getByRole('button', { name: 'Add a Label' }));
await user.type(awsRoleArn(), '*');
await user.type(azureIdentity(), '*');
await user.type(gcpServiceAccount(), '*');
await user.click(
within(awsRoleArns()).getByRole('button', { name: 'Add More' })
);
await user.type(awsRoleArnTextBoxes()[1], '*');
await user.click(
within(azureIdentities()).getByRole('button', { name: 'Add More' })
);
await user.type(azureIdentityTextBoxes()[1], '*');
await user.click(
within(gcpServiceAccounts()).getByRole('button', { name: 'Add More' })
);
await user.type(gcpServiceAccountTextBoxes()[1], '*');
act(() => validator.validate());
expect(
screen.getByPlaceholderText('label key')
).toHaveAccessibleDescription('required');
expect(awsRoleArn()).toHaveAccessibleDescription(
expect(awsRoleArnTextBoxes()[1]).toHaveAccessibleDescription(
'Wildcard is not allowed in AWS role ARNs'
);
expect(azureIdentity()).toHaveAccessibleDescription(
expect(azureIdentityTextBoxes()[1]).toHaveAccessibleDescription(
'Wildcard is not allowed in Azure identities'
);
expect(gcpServiceAccount()).toHaveAccessibleDescription(
expect(gcpServiceAccountTextBoxes()[1]).toHaveAccessibleDescription(
'Wildcard is not allowed in GCP service accounts'
);
});
Expand Down Expand Up @@ -521,9 +554,18 @@ describe('DatabaseAccessSpecSection', () => {
expect(onChange).toHaveBeenLastCalledWith({
kind: 'db',
labels: [{ name: 'env', value: 'prod' }],
names: [expect.objectContaining({ label: 'stuff', value: 'stuff' })],
roles: [expect.objectContaining({ label: 'admin', value: 'admin' })],
users: [expect.objectContaining({ label: 'mary', value: 'mary' })],
names: [
expect.objectContaining({ value: '{{internal.db_names}}' }),
expect.objectContaining({ label: 'stuff', value: 'stuff' }),
],
roles: [
expect.objectContaining({ value: '{{internal.db_roles}}' }),
expect.objectContaining({ label: 'admin', value: 'admin' }),
],
users: [
expect.objectContaining({ value: '{{internal.db_users}}' }),
expect.objectContaining({ label: 'mary', value: 'mary' }),
],
} as DatabaseAccessSpec);
});

Expand Down Expand Up @@ -572,7 +614,10 @@ describe('WindowsDesktopAccessSpecSection', () => {
expect(onChange).toHaveBeenLastCalledWith({
kind: 'windows_desktop',
labels: [{ name: 'os', value: 'win-xp' }],
logins: [expect.objectContaining({ label: 'julio', value: 'julio' })],
logins: [
expect.objectContaining({ value: '{{internal.windows_logins}}' }),
expect.objectContaining({ label: 'julio', value: 'julio' }),
],
} as WindowsDesktopAccessSpec);
});

Expand Down
Loading

0 comments on commit 5ebc5bf

Please sign in to comment.