From 44925dd8ad3a1d0a494ca019eda5e198af3d61dc Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 6 Jun 2024 16:13:35 -0400 Subject: [PATCH 01/29] editor mockup --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 121 +++++++++++++++++- .../src/Users/UserAddEdit/useDialog.tsx | 1 + 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index cefb4c64a3528..257529232e271 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -16,8 +16,18 @@ * along with this program. If not, see . */ -import React from 'react'; -import { ButtonPrimary, ButtonSecondary, Alert } from 'design'; +import React, { useState, useEffect } from 'react'; +import { + ButtonPrimary, + ButtonSecondary, + Alert, + Box, + Flex, + Text, + ButtonIcon, +} from 'design'; +import { ButtonTextWithAddIcon } from 'shared/components/ButtonTextWithAddIcon'; +import * as Icons from 'design/Icon'; import Dialog, { DialogHeader, DialogTitle, @@ -26,10 +36,15 @@ import Dialog, { } from 'design/Dialog'; import Validation from 'shared/components/Validation'; import FieldInput from 'shared/components/FieldInput'; -import { FieldSelectAsync } from 'shared/components/FieldSelect'; +import { + FieldSelectAsync, + FieldSelectCreatable, +} from 'shared/components/FieldSelect'; import { Option } from 'shared/components/Select'; import { requiredField } from 'shared/components/Validation/rules'; +import { AllUserTraits } from 'teleport/services/user'; + import UserTokenLink from './../UserTokenLink'; import useDialog, { Props } from './useDialog'; @@ -44,6 +59,7 @@ export function UserAddEdit(props: ReturnType) { onChangeRoles, onClose, fetchRoles, + traits, attempt, name, selectedRoles, @@ -69,9 +85,9 @@ export function UserAddEdit(props: ReturnType) { {({ validator }) => ( ({ - maxWidth: '500px', + maxWidth: '700px', width: '100%', - overflow: 'initial', + height: '70%', })} disableEscapeKeyDown={false} onClose={onClose} @@ -111,6 +127,7 @@ export function UserAddEdit(props: ReturnType) { }} elevated={true} /> + ) { ); } + +export type TraitEditorProps = { + traits: AllUserTraits; +}; + +function TraitsEditor({ traits }: TraitEditorProps) { + const [tt, setTT] = useState([]); + + useEffect(() => { + let t = []; + for (let trait in traits) { + if (traits[trait].length !== 0) { + t.push({ trait: trait, traitValues: traits[trait] }); + } + } + console.log(t); + setTT(t); + }, [traits]); + + return ( + + Traits + + + Trait Name + + + + Trait Value + + + {tt.map(({ trait, traitValues }) => { + return ( + + + + + + + props.theme.colors.levels.surface}; + `} + placeholder="trait values" + defaultValue={traitValues.map(r => ({ + value: r, + label: r, + }))} + isMulti + value={traitValues.map(r => ({ + value: r, + label: r, + }))} + isDisabled={false} + createOptionPosition="last" + formatCreateLabel={(i: string) => `"${i}"`} + /> + + null} + css={` + &:disabled { + opacity: 0.65; + pointer-events: none; + } + `} + disabled={false} + > + + + + + ); + })} + + + null} + label={'Add another trait'} + disabled={false} + /> + + + ); +} diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 8c65c90b9178f..3fcb6dc6230d1 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -76,6 +76,7 @@ export default function useUserDialog(props: Props) { onChangeRoles, fetchRoles: props.fetchRoles, isNew: props.isNew, + traits: props.user.allTraits, attempt, name, selectedRoles, From 67a92fc8f38f479b46030f53d7f73614d0589ea9 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 6 Jun 2024 17:55:17 -0400 Subject: [PATCH 02/29] add empty traits to available traits so user can select from dropdown --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 257529232e271..5aff2ddd64e21 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -59,7 +59,7 @@ export function UserAddEdit(props: ReturnType) { onChangeRoles, onClose, fetchRoles, - traits, + allTraits, attempt, name, selectedRoles, @@ -127,7 +127,7 @@ export function UserAddEdit(props: ReturnType) { }} elevated={true} /> - + ) { } export type TraitEditorProps = { - traits: AllUserTraits; + allTraits: AllUserTraits; }; -function TraitsEditor({ traits }: TraitEditorProps) { +function TraitsEditor({ allTraits }: TraitEditorProps) { const [tt, setTT] = useState([]); + const [availableTraits, setAvailableTraits] = useState([]) useEffect(() => { + console.log('allTraits: ', allTraits) let t = []; - for (let trait in traits) { - if (traits[trait].length !== 0) { - t.push({ trait: trait, traitValues: traits[trait] }); + for (let trait in allTraits) { + if (allTraits[trait].length === 0) { + // send empty traits to availableTraits so that users can choose from Trait Name dropdown. + availableTraits.push({value: trait, label: trait}) + setAvailableTraits(availableTraits) + } + if (allTraits[trait].length !== 0) { + t.push({ trait: trait, traitValues: allTraits[trait] }); } } console.log(t); setTT(t); - }, [traits]); + }, [allTraits]); return ( @@ -186,6 +193,7 @@ function TraitsEditor({ traits }: TraitEditorProps) { Date: Fri, 7 Jun 2024 10:27:11 -0400 Subject: [PATCH 03/29] trait name and value editor --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 80 +++++++++++++++---- .../src/Users/UserAddEdit/useDialog.tsx | 2 +- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 5aff2ddd64e21..8f09d2996d641 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -155,26 +155,59 @@ export type TraitEditorProps = { }; function TraitsEditor({ allTraits }: TraitEditorProps) { - const [tt, setTT] = useState([]); + const [traits, setTraits] = useState([]); - const [availableTraits, setAvailableTraits] = useState([]) + const [availableTraits, setAvailableTraits] = useState([]); useEffect(() => { - console.log('allTraits: ', allTraits) let t = []; for (let trait in allTraits) { if (allTraits[trait].length === 0) { // send empty traits to availableTraits so that users can choose from Trait Name dropdown. - availableTraits.push({value: trait, label: trait}) - setAvailableTraits(availableTraits) + availableTraits.push({ value: trait, label: trait }); + setAvailableTraits(availableTraits); } - if (allTraits[trait].length !== 0) { - t.push({ trait: trait, traitValues: allTraits[trait] }); + if (!allTraits[trait][0]) { + continue; + } + if (allTraits[trait].length > 0) { + t.push({ + trait: trait, + traitValues: allTraits[trait].map(t => ({ + value: t, + label: t, + })), + }); } } - console.log(t); - setTT(t); + setTraits(t); }, [allTraits]); + type InputOption = { + labelField: 'trait' | 'traitValues'; + option: Option | Option[]; + index: number; + }; + + function handleInputChange(i: InputOption) { + if (i.labelField === 'traitValues') { + let traitValue: Option[] = i.option as Option[]; + const newList = [...traits]; + newList[i.index] = { + ...newList[i.index], + [i.labelField]: [...traitValue], + }; + setTraits(newList); + } else { + let traitName: Option = i.option as Option; + const newList = [...traits]; + newList[i.index] = { + ...newList[i.index], + [i.labelField]: traitName.value, + }; + setTraits(newList); + } + } + return ( Traits @@ -187,22 +220,30 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { Trait Value - {tt.map(({ trait, traitValues }) => { + {traits.map(({ trait, traitValues }, index) => { return ( { + handleInputChange({ + option: e as Option, + labelField: 'trait', + index: index, + }); + }} /> props.theme.colors.levels.surface}; `} @@ -212,10 +253,15 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { label: r, }))} isMulti - value={traitValues.map(r => ({ - value: r, - label: r, - }))} + isSearchable + value={traitValues} + onChange={e => { + handleInputChange({ + option: e as Option, + labelField: 'traitValues', + index: index, + }); + }} isDisabled={false} createOptionPosition="last" formatCreateLabel={(i: string) => `"${i}"`} @@ -224,7 +270,7 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { null} css={` &:disabled { diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 3fcb6dc6230d1..5c929e49fde39 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -76,7 +76,7 @@ export default function useUserDialog(props: Props) { onChangeRoles, fetchRoles: props.fetchRoles, isNew: props.isNew, - traits: props.user.allTraits, + allTraits: props.user.allTraits, attempt, name, selectedRoles, From b9ae40819ad00b93c68c704987e2ae17983cd2ec Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Fri, 7 Jun 2024 12:45:20 -0400 Subject: [PATCH 04/29] transform editor data to match with onSave func --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 180 ++++++++++-------- .../src/Users/UserAddEdit/useDialog.tsx | 10 + 2 files changed, 110 insertions(+), 80 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 8f09d2996d641..58aa6f38311df 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, Dispatch, SetStateAction } from 'react'; import { ButtonPrimary, ButtonSecondary, @@ -48,6 +48,8 @@ import { AllUserTraits } from 'teleport/services/user'; import UserTokenLink from './../UserTokenLink'; import useDialog, { Props } from './useDialog'; +import type { TraitEditor } from './useDialog'; + export default function Container(props: Props) { const dialog = useDialog(props); return ; @@ -59,6 +61,7 @@ export function UserAddEdit(props: ReturnType) { onChangeRoles, onClose, fetchRoles, + setConfiguredTraits, allTraits, attempt, name, @@ -66,6 +69,7 @@ export function UserAddEdit(props: ReturnType) { onSave, isNew, token, + configuredTraits, } = props; if (attempt.status === 'success' && isNew) { @@ -127,7 +131,11 @@ export function UserAddEdit(props: ReturnType) { }} elevated={true} /> - + ) { export type TraitEditorProps = { allTraits: AllUserTraits; + setConfiguredTraits: Dispatch>; + configuredTraits: TraitEditor[]; }; -function TraitsEditor({ allTraits }: TraitEditorProps) { - const [traits, setTraits] = useState([]); +function TraitsEditor({ + allTraits, + configuredTraits, + setConfiguredTraits, +}: TraitEditorProps) { + // const [traits, setTraits] = useState([]); - const [availableTraits, setAvailableTraits] = useState([]); + const [availableTraitNames, setAvailableTraitNames] = useState([]); useEffect(() => { - let t = []; + console.log('allTraits: ', allTraits); + console.log('allTraits: ', JSON.stringify(allTraits)); + let newTrait = []; for (let trait in allTraits) { if (allTraits[trait].length === 0) { // send empty traits to availableTraits so that users can choose from Trait Name dropdown. - availableTraits.push({ value: trait, label: trait }); - setAvailableTraits(availableTraits); + availableTraitNames.push({ value: trait, label: trait }); + setAvailableTraitNames(availableTraitNames); } if (!allTraits[trait][0]) { continue; } if (allTraits[trait].length > 0) { - t.push({ - trait: trait, + newTrait.push({ + trait: { value: trait, label: trait }, traitValues: allTraits[trait].map(t => ({ value: t, label: t, @@ -179,7 +195,8 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { }); } } - setTraits(t); + + setConfiguredTraits(newTrait); }, [allTraits]); type InputOption = { @@ -191,20 +208,20 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { function handleInputChange(i: InputOption) { if (i.labelField === 'traitValues') { let traitValue: Option[] = i.option as Option[]; - const newList = [...traits]; + const newList = [...configuredTraits]; newList[i.index] = { ...newList[i.index], [i.labelField]: [...traitValue], }; - setTraits(newList); + setConfiguredTraits(newList); } else { let traitName: Option = i.option as Option; - const newList = [...traits]; + const newList = [...configuredTraits]; newList[i.index] = { ...newList[i.index], - [i.labelField]: traitName.value, + [i.labelField]: traitName, }; - setTraits(newList); + setConfiguredTraits(newList); } } @@ -220,72 +237,75 @@ function TraitsEditor({ allTraits }: TraitEditorProps) { Trait Value - {traits.map(({ trait, traitValues }, index) => { - return ( - - - - { - handleInputChange({ - option: e as Option, - labelField: 'trait', - index: index, - }); - }} - /> - - - + {configuredTraits.map(({ trait, traitValues }, index) => { + return ( + + + + { + handleInputChange({ + option: e as Option, + labelField: 'trait', + index: index, + }); + }} + /> + + + props.theme.colors.levels.surface}; + `} + placeholder="trait values" + defaultValue={traitValues.map(r => ({ + value: r, + label: r, + }))} + isMulti + isSearchable + value={traitValues} + onChange={e => { + handleInputChange({ + option: e as Option, + labelField: 'traitValues', + index: index, + }); + }} + isDisabled={false} + createOptionPosition="last" + formatCreateLabel={(i: string) => `"${i}"`} + /> + + null} css={` - background: ${props => props.theme.colors.levels.surface}; + &:disabled { + opacity: 0.65; + pointer-events: none; + } `} - placeholder="trait values" - defaultValue={traitValues.map(r => ({ - value: r, - label: r, - }))} - isMulti - isSearchable - value={traitValues} - onChange={e => { - handleInputChange({ - option: e as Option, - labelField: 'traitValues', - index: index, - }); - }} - isDisabled={false} - createOptionPosition="last" - formatCreateLabel={(i: string) => `"${i}"`} - /> - - null} - css={` - &:disabled { - opacity: 0.65; - pointer-events: none; - } - `} - disabled={false} - > - - - - - ); - })} + disabled={false} + > + + + + + ); + })} + ([]); function onChangeName(name = '') { setName(name); @@ -42,9 +45,14 @@ export default function useUserDialog(props: Props) { } function onSave() { + const traitsToSave = {}; + for (const trait of configuredTraits) { + traitsToSave[trait.trait.value] = trait.traitValues.map(t => t.value); + } const u = { name, roles: selectedRoles.map(r => r.value), + allTraits: traitsToSave, }; const handleError = (err: Error) => @@ -75,12 +83,14 @@ export default function useUserDialog(props: Props) { onChangeName, onChangeRoles, fetchRoles: props.fetchRoles, + setConfiguredTraits, isNew: props.isNew, allTraits: props.user.allTraits, attempt, name, selectedRoles, token, + configuredTraits, }; } From e7616afdcc90d1c4fcf9f1b40691e4dfa8ac739a Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Fri, 7 Jun 2024 16:46:52 -0400 Subject: [PATCH 05/29] add new trait --- .../teleport/src/Users/UserAddEdit/UserAddEdit.tsx | 8 +++++++- web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 58aa6f38311df..fcc8d0097e8ed 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -225,6 +225,12 @@ function TraitsEditor({ } } + function addTrait() { + const newList = [...configuredTraits]; + newList.push({trait: {value: '', label: ''}, traitValues: []}) + setConfiguredTraits(newList) + } + return ( Traits @@ -309,7 +315,7 @@ function TraitsEditor({ null} + onClick={addTrait} label={'Add another trait'} disabled={false} /> diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 031a8c4aa1c86..c4d8e27ff73d1 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -47,6 +47,9 @@ export default function useUserDialog(props: Props) { function onSave() { const traitsToSave = {}; for (const trait of configuredTraits) { + if (trait.trait.value === '' || trait.traitValues.length === 0) { + continue + } traitsToSave[trait.trait.value] = trait.traitValues.map(t => t.value); } const u = { From aad4b5fdfba273ffc8d161e2e2f496f2e3df7bf1 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Fri, 7 Jun 2024 16:53:09 -0400 Subject: [PATCH 06/29] remove trait --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index fcc8d0097e8ed..20bdb3c16f519 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -169,12 +169,9 @@ function TraitsEditor({ configuredTraits, setConfiguredTraits, }: TraitEditorProps) { - // const [traits, setTraits] = useState([]); const [availableTraitNames, setAvailableTraitNames] = useState([]); useEffect(() => { - console.log('allTraits: ', allTraits); - console.log('allTraits: ', JSON.stringify(allTraits)); let newTrait = []; for (let trait in allTraits) { if (allTraits[trait].length === 0) { @@ -206,29 +203,35 @@ function TraitsEditor({ }; function handleInputChange(i: InputOption) { + const newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { let traitValue: Option[] = i.option as Option[]; - const newList = [...configuredTraits]; - newList[i.index] = { - ...newList[i.index], + + newTraits[i.index] = { + ...newTraits[i.index], [i.labelField]: [...traitValue], }; - setConfiguredTraits(newList); + setConfiguredTraits(newTraits); } else { let traitName: Option = i.option as Option; - const newList = [...configuredTraits]; - newList[i.index] = { - ...newList[i.index], + newTraits[i.index] = { + ...newTraits[i.index], [i.labelField]: traitName, }; - setConfiguredTraits(newList); + setConfiguredTraits(newTraits); } } function addTrait() { - const newList = [...configuredTraits]; - newList.push({trait: {value: '', label: ''}, traitValues: []}) - setConfiguredTraits(newList) + const newTraits = [...configuredTraits]; + newTraits.push({trait: {value: '', label: ''}, traitValues: []}) + setConfiguredTraits(newTraits) + } + + function removeTrait(index: number) { + const newTraits = [...configuredTraits]; + newTraits.splice(index, 1); + setConfiguredTraits(newTraits) } return ( @@ -296,7 +299,7 @@ function TraitsEditor({ ml={1} size={1} title="Remove Trait" - onClick={() => null} + onClick={() => removeTrait(index)} css={` &:disabled { opacity: 0.65; From ecec97659164fae918742a682565aa02f91a6bae Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Fri, 7 Jun 2024 16:54:41 -0400 Subject: [PATCH 07/29] add available trait name list --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 20bdb3c16f519..82bc3505b5aaf 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -170,15 +170,24 @@ function TraitsEditor({ setConfiguredTraits, }: TraitEditorProps) { - const [availableTraitNames, setAvailableTraitNames] = useState([]); + const availableTraitNames = [ + 'aws_role_arns', + 'azure_identities', + 'db_names', + 'db_roles', + 'db_users', + 'gcp_service_accounts', + 'host_user_gid', + 'host_user_uid', + 'kubernetes_groups', + 'kubernetes_users', + 'logins', + 'windows_logins' + ] + useEffect(() => { let newTrait = []; for (let trait in allTraits) { - if (allTraits[trait].length === 0) { - // send empty traits to availableTraits so that users can choose from Trait Name dropdown. - availableTraitNames.push({ value: trait, label: trait }); - setAvailableTraitNames(availableTraitNames); - } if (!allTraits[trait][0]) { continue; } @@ -254,7 +263,10 @@ function TraitsEditor({ ({ + value: r, + label: r, + }))} placeholder="trait-key" autoFocus isSearchable From 30a133dd48bc91cfb29784b67bd83080c0e5ab2c Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Fri, 7 Jun 2024 17:35:40 -0400 Subject: [PATCH 08/29] show traits header only when trait count is more than zero --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 47 ++++++++++--------- .../src/Users/UserAddEdit/useDialog.tsx | 2 +- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 82bc3505b5aaf..f9f22073bd2f6 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import React, { useState, useEffect, Dispatch, SetStateAction } from 'react'; +import React, { useEffect, Dispatch, SetStateAction } from 'react'; import { ButtonPrimary, ButtonSecondary, @@ -169,7 +169,6 @@ function TraitsEditor({ configuredTraits, setConfiguredTraits, }: TraitEditorProps) { - const availableTraitNames = [ 'aws_role_arns', 'azure_identities', @@ -182,8 +181,8 @@ function TraitsEditor({ 'kubernetes_groups', 'kubernetes_users', 'logins', - 'windows_logins' - ] + 'windows_logins', + ]; useEffect(() => { let newTrait = []; @@ -233,28 +232,34 @@ function TraitsEditor({ function addTrait() { const newTraits = [...configuredTraits]; - newTraits.push({trait: {value: '', label: ''}, traitValues: []}) - setConfiguredTraits(newTraits) + newTraits.push({ trait: { value: '', label: '' }, traitValues: [] }); + setConfiguredTraits(newTraits); } function removeTrait(index: number) { const newTraits = [...configuredTraits]; newTraits.splice(index, 1); - setConfiguredTraits(newTraits) + setConfiguredTraits(newTraits); } + const addLabelText = + configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; return ( - Traits - - - Trait Name - + {configuredTraits.length > 0 && ( + <> + Traits + + + Trait Name + - - Trait Value - - + + Trait Value + + + + )} {configuredTraits.map(({ trait, traitValues }, index) => { @@ -267,10 +272,10 @@ function TraitsEditor({ value: r, label: r, }))} - placeholder="trait-key" + placeholder="Select or type new trait name" autoFocus isSearchable - value={trait} + value={trait.value === '' ? null : trait} onChange={e => { handleInputChange({ option: e as Option, @@ -287,13 +292,14 @@ function TraitsEditor({ css={` background: ${props => props.theme.colors.levels.surface}; `} - placeholder="trait values" + placeholder="Type a new trait value and enter" defaultValue={traitValues.map(r => ({ value: r, label: r, }))} isMulti isSearchable + isClearable={false} value={traitValues} onChange={e => { handleInputChange({ @@ -304,7 +310,6 @@ function TraitsEditor({ }} isDisabled={false} createOptionPosition="last" - formatCreateLabel={(i: string) => `"${i}"`} /> diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index c4d8e27ff73d1..06e164b68f7b2 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -48,7 +48,7 @@ export default function useUserDialog(props: Props) { const traitsToSave = {}; for (const trait of configuredTraits) { if (trait.trait.value === '' || trait.traitValues.length === 0) { - continue + continue; } traitsToSave[trait.trait.value] = trait.traitValues.map(t => t.value); } From 715a04c07d3aeb98ccc5151f05cc5d55a27ac55e Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 06:39:24 -0400 Subject: [PATCH 09/29] add duplicate trait rule, add requireAll wrapper to shared rules --- .../shared/components/Validation/rules.ts | 29 ++++++++++ .../src/Users/UserAddEdit/UserAddEdit.tsx | 53 ++++++++++++------- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/web/packages/shared/components/Validation/rules.ts b/web/packages/shared/components/Validation/rules.ts index 84a7802000754..456782f10955b 100644 --- a/web/packages/shared/components/Validation/rules.ts +++ b/web/packages/shared/components/Validation/rules.ts @@ -194,6 +194,34 @@ const requiredEmailLike: Rule = email => () => { }; }; +/** + * A rule function that combines multiple inner rule functions. All rules must + * return `valid`, otherwise it returns a comma separated string containing all + * invalid rule messages. + * @param rules a list of rule functions to apply + * @returns a rule function that ANDs all input rules + */ +const requiredAll = + (...rules: Rule[]): Rule => + (value: T) => + () => { + let messages = []; + for (let r of rules) { + let result = r(value)(); + if (!result.valid) { + messages.push(result.message); + } + } + + if (messages.length > 0) { + return { + valid: false, + message: messages.join('. '), + }; + } + return { valid: true }; + }; + export { requiredToken, requiredPassword, @@ -202,4 +230,5 @@ export { requiredRoleArn, requiredIamRoleName, requiredEmailLike, + requiredAll, }; diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index f9f22073bd2f6..5309ebc75f04b 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -41,7 +41,7 @@ import { FieldSelectCreatable, } from 'shared/components/FieldSelect'; import { Option } from 'shared/components/Select'; -import { requiredField } from 'shared/components/Validation/rules'; +import { requiredField, requiredAll } from 'shared/components/Validation/rules'; import { AllUserTraits } from 'teleport/services/user'; @@ -81,6 +81,8 @@ export function UserAddEdit(props: ReturnType) { return; } + console.log('returning without save'); + return; onSave(); } @@ -211,6 +213,7 @@ function TraitsEditor({ }; function handleInputChange(i: InputOption) { + // validator.reset() const newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { let traitValue: Option[] = i.option as Option[]; @@ -222,6 +225,7 @@ function TraitsEditor({ setConfiguredTraits(newTraits); } else { let traitName: Option = i.option as Option; + newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitName, @@ -232,7 +236,10 @@ function TraitsEditor({ function addTrait() { const newTraits = [...configuredTraits]; - newTraits.push({ trait: { value: '', label: '' }, traitValues: [] }); + newTraits.push({ + trait: { value: '', label: 'Select or type new trait name and enter' }, + traitValues: [], + }); setConfiguredTraits(newTraits); } @@ -244,28 +251,30 @@ function TraitsEditor({ const addLabelText = configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; + + const requireNoDuplicateTraits = (enteredTrait: Option) => () => { + let k = configuredTraits.map(trait => trait.trait.value.toLowerCase()); + let occurance = 0; + for (let t in k) { + if (k[t] === enteredTrait.value.toLowerCase()) { + occurance++; + } + } + if (occurance > 1) { + return { valid: false, message: 'Trait key should be unique for a user' }; + } + return { valid: true }; + }; + return ( - {configuredTraits.length > 0 && ( - <> - Traits - - - Trait Name - - - - Trait Value - - - - )} + {configuredTraits.length > 0 && Traits} {configuredTraits.map(({ trait, traitValues }, index) => { return ( - + ({ @@ -275,7 +284,12 @@ function TraitsEditor({ placeholder="Select or type new trait name" autoFocus isSearchable - value={trait.value === '' ? null : trait} + value={trait} + label="Trait Name" + rule={requiredAll( + requiredField('Trait key is required'), + requireNoDuplicateTraits + )} onChange={e => { handleInputChange({ option: e as Option, @@ -297,10 +311,12 @@ function TraitsEditor({ value: r, label: r, }))} + label="Trait Value" isMulti isSearchable isClearable={false} value={traitValues} + rule={requiredField('Trait value cannot be empty')} onChange={e => { handleInputChange({ option: e as Option, @@ -314,6 +330,7 @@ function TraitsEditor({ removeTrait(index)} From 8001bdd0c2463ed0f12e194f931c4e091a87a22f Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 08:23:21 -0400 Subject: [PATCH 10/29] update editor design --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 5309ebc75f04b..0f92598f725e7 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -20,14 +20,14 @@ import React, { useEffect, Dispatch, SetStateAction } from 'react'; import { ButtonPrimary, ButtonSecondary, + ButtonBorder, Alert, Box, Flex, Text, ButtonIcon, } from 'design'; -import { ButtonTextWithAddIcon } from 'shared/components/ButtonTextWithAddIcon'; -import * as Icons from 'design/Icon'; +import { Add, Trash } from 'design/Icon'; import Dialog, { DialogHeader, DialogTitle, @@ -102,11 +102,12 @@ export function UserAddEdit(props: ReturnType) { {isNew ? 'Create User' : 'Edit User'} - + {attempt.status === 'failed' && ( )} ) { readonly={isNew ? false : true} /> - {configuredTraits.length > 0 && Traits} + User Traits {configuredTraits.map(({ trait, traitValues }, index) => { @@ -285,7 +287,7 @@ function TraitsEditor({ autoFocus isSearchable value={trait} - label="Trait Name" + label="Key" rule={requiredAll( requiredField('Trait key is required'), requireNoDuplicateTraits @@ -297,6 +299,7 @@ function TraitsEditor({ index: index, }); }} + createOptionPosition="last" /> @@ -311,7 +314,7 @@ function TraitsEditor({ value: r, label: r, }))} - label="Trait Value" + label="Value" isMulti isSearchable isClearable={false} @@ -325,7 +328,9 @@ function TraitsEditor({ }); }} isDisabled={false} - createOptionPosition="last" + formatCreateLabel={(i: string) => + 'Trait value: ' + `"${i}"` + } /> - + @@ -351,11 +356,29 @@ function TraitsEditor({ - + > + + {addLabelText} + ); From 2a914e91899e20b1929f3e8db40922d781b38c38 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 11:26:35 -0400 Subject: [PATCH 11/29] move TraitsEditor to separate file, add unit test --- .../Users/UserAddEdit/TraitsEditor.test.tsx | 88 ++++++ .../src/Users/UserAddEdit/TraitsEditor.tsx | 259 ++++++++++++++++++ .../Users/UserAddEdit/UserAddEdit.story.tsx | 12 + .../src/Users/UserAddEdit/UserAddEdit.tsx | 247 +---------------- .../src/Users/UserAddEdit/useDialog.tsx | 4 +- 5 files changed, 366 insertions(+), 244 deletions(-) create mode 100644 web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx create mode 100644 web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx new file mode 100644 index 0000000000000..cce729f26b0e3 --- /dev/null +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -0,0 +1,88 @@ +import React from 'react'; +import { fireEvent, render, screen } from 'design/utils/testing'; + +import Validation from 'shared/components/Validation'; + +import { AllUserTraits } from 'teleport/services/user'; + +import { TraitsEditor, traitsToTraitsOption, emptyTrait } from './TraitsEditor'; + +import type { TraitsOption } from './TraitsEditor'; + +describe('Render traits correctly', () => { + const userTraits = { + logins: ['root', 'ubuntu'], + db_roles: ['dbadmin', 'postgres'], + db_names: ['postgres', 'aurora'], + } as AllUserTraits; + + test('Avalable traits are rendered', async () => { + let configuredTraits = [] as TraitsOption[]; + + const setConfiguredTraits = jest.fn(); + + const { rerender } = render( + + ); + + expect(screen.getByText('User Traits')).toBeInTheDocument(); + + expect(setConfiguredTraits).toHaveBeenLastCalledWith( + traitsToTraitsOption(userTraits) + ); + + rerender( + + + + ); + expect(screen.getAllByTestId('trait-key')).toHaveLength(3); + expect(screen.getAllByTestId('trait-value')).toHaveLength(3); + }); + + test('Add and remove Trait', async () => { + const userTraits = {} as AllUserTraits; + + let configuredTraits = [] as TraitsOption[]; + + const setConfiguredTraits = jest.fn(); + + const { rerender } = render( + + + + ); + expect(screen.queryAllByTestId('trait-key')).toHaveLength(0); + + const addButtonEl = screen.getByRole('button', { name: /Add user trait/i }); + expect(addButtonEl).toBeInTheDocument(); + fireEvent.click(addButtonEl); + + expect(setConfiguredTraits).toHaveBeenLastCalledWith([emptyTrait]); + + const singleTrait = { logins: ['root', 'ubuntu'] }; + rerender( + + + + ); + fireEvent.click(screen.getByTitle('Remove Trait')); + expect(setConfiguredTraits).toHaveBeenCalledWith([]); + }); +}); diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx new file mode 100644 index 0000000000000..ba04ec4299f51 --- /dev/null +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -0,0 +1,259 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import React, { useEffect, Dispatch, SetStateAction } from 'react'; +import { ButtonBorder, Box, Flex, Text, ButtonIcon } from 'design'; +import { Add, Trash } from 'design/Icon'; +import { FieldSelectCreatable } from 'shared/components/FieldSelect'; +import { Option } from 'shared/components/Select'; +import { requiredField, requiredAll } from 'shared/components/Validation/rules'; + +import { AllUserTraits } from 'teleport/services/user'; + +const availableTraitNames = [ + 'aws_role_arns', + 'azure_identities', + 'db_names', + 'db_roles', + 'db_users', + 'gcp_service_accounts', + 'host_user_gid', + 'host_user_uid', + 'kubernetes_groups', + 'kubernetes_users', + 'logins', + 'windows_logins', +]; + +export function TraitsEditor({ + allTraits, + configuredTraits, + setConfiguredTraits, +}: TraitEditorProps) { + useEffect(() => { + let newTrait = traitsToTraitsOption(allTraits); + + setConfiguredTraits(newTrait); + }, [allTraits]); + + type InputOption = { + labelField: 'trait' | 'traitValues'; + option: Option | Option[]; + index: number; + }; + + function handleInputChange(i: InputOption) { + const newTraits = [...configuredTraits]; + if (i.labelField === 'traitValues') { + let traitValue: Option[] = i.option as Option[]; + + newTraits[i.index] = { + ...newTraits[i.index], + [i.labelField]: [...traitValue], + }; + setConfiguredTraits(newTraits); + } else { + let traitName: Option = i.option as Option; + + newTraits[i.index] = { + ...newTraits[i.index], + [i.labelField]: traitName, + }; + setConfiguredTraits(newTraits); + } + } + + function addTrait() { + const newTraits = [...configuredTraits]; + newTraits.push(emptyTrait); + setConfiguredTraits(newTraits); + } + + function removeTrait(index: number) { + const newTraits = [...configuredTraits]; + newTraits.splice(index, 1); + setConfiguredTraits(newTraits); + } + + const addLabelText = + configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; + + const requireNoDuplicateTraits = (enteredTrait: Option) => () => { + let k = configuredTraits.map(trait => trait.trait.value.toLowerCase()); + let occurance = 0; + for (let t in k) { + if (k[t] === enteredTrait.value.toLowerCase()) { + occurance++; + } + } + if (occurance > 1) { + return { valid: false, message: 'Trait key should be unique for a user' }; + } + return { valid: true }; + }; + + return ( + + User Traits + + + {configuredTraits.map(({ trait, traitValues }, index) => { + return ( + + + + ({ + value: r, + label: r, + }))} + placeholder="Select or type new trait name and enter" + autoFocus + isSearchable + value={trait} + label="Key" + rule={requiredAll( + requiredField('Trait key is required'), + requireNoDuplicateTraits + )} + onChange={e => { + handleInputChange({ + option: e as Option, + labelField: 'trait', + index: index, + }); + }} + createOptionPosition="last" + /> + + + props.theme.colors.levels.surface}; + `} + placeholder="Type a new trait value and enter" + defaultValue={traitValues.map(r => ({ + value: r, + label: r, + }))} + label="Value" + isMulti + isSearchable + isClearable={false} + value={traitValues} + rule={requiredField('Trait value cannot be empty')} + onChange={e => { + handleInputChange({ + option: e as Option, + labelField: 'traitValues', + index: index, + }); + }} + isDisabled={false} + formatCreateLabel={(i: string) => + 'Trait value: ' + `"${i}"` + } + /> + + removeTrait(index)} + css={` + &:disabled { + opacity: 0.65; + pointer-events: none; + } + `} + disabled={false} + > + + + + + ); + })} + + + + + + {addLabelText} + + + + ); +} + +export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { + let newTrait = []; + for (let trait in allTraits) { + if (!allTraits[trait][0]) { + continue; + } + if (allTraits[trait].length > 0) { + newTrait.push({ + trait: { value: trait, label: trait }, + traitValues: allTraits[trait].map(t => ({ + value: t, + label: t, + })), + }); + } + } + return newTrait; +} + +export const emptyTrait = { + trait: { value: '', label: 'Select or type new trait name and enter' }, + traitValues: [], +}; + +export type TraitsOption = { trait: Option; traitValues: Option[] }; + +export type TraitEditorProps = { + allTraits: AllUserTraits; + setConfiguredTraits: Dispatch>; + configuredTraits: TraitsOption[]; +}; diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx index 67a316c4e2bb8..dbdb528ec532f 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx @@ -18,8 +18,12 @@ import React from 'react'; +import { AllUserTraits } from 'teleport/services/user'; + import { UserAddEdit } from './UserAddEdit'; +import type { TraitsOption } from './TraitsEditor'; + export default { title: 'Teleport/Users/UserAddEdit', }; @@ -72,4 +76,12 @@ const props = { expires: new Date('2050-12-20T17:28:20.93Z'), username: 'Lester', }, + allTraits: { ['logins']: ['root', 'ubuntu'] } as AllUserTraits, + configuredTraits: [ + { + trait: { value: 'logins', label: 'logins' }, + traitValues: [{ value: 'root', label: 'root' }], + }, + ] as TraitsOption[], + setConfiguredTraits: () => null, }; diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index 0f92598f725e7..b0e8fc303c184 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -16,18 +16,8 @@ * along with this program. If not, see . */ -import React, { useEffect, Dispatch, SetStateAction } from 'react'; -import { - ButtonPrimary, - ButtonSecondary, - ButtonBorder, - Alert, - Box, - Flex, - Text, - ButtonIcon, -} from 'design'; -import { Add, Trash } from 'design/Icon'; +import React from 'react'; +import { ButtonPrimary, ButtonSecondary, Alert } from 'design'; import Dialog, { DialogHeader, DialogTitle, @@ -36,19 +26,14 @@ import Dialog, { } from 'design/Dialog'; import Validation from 'shared/components/Validation'; import FieldInput from 'shared/components/FieldInput'; -import { - FieldSelectAsync, - FieldSelectCreatable, -} from 'shared/components/FieldSelect'; +import { FieldSelectAsync } from 'shared/components/FieldSelect'; import { Option } from 'shared/components/Select'; -import { requiredField, requiredAll } from 'shared/components/Validation/rules'; - -import { AllUserTraits } from 'teleport/services/user'; +import { requiredField } from 'shared/components/Validation/rules'; import UserTokenLink from './../UserTokenLink'; import useDialog, { Props } from './useDialog'; -import type { TraitEditor } from './useDialog'; +import { TraitsEditor } from './TraitsEditor'; export default function Container(props: Props) { const dialog = useDialog(props); @@ -161,225 +146,3 @@ export function UserAddEdit(props: ReturnType) { ); } - -export type TraitEditorProps = { - allTraits: AllUserTraits; - setConfiguredTraits: Dispatch>; - configuredTraits: TraitEditor[]; -}; - -function TraitsEditor({ - allTraits, - configuredTraits, - setConfiguredTraits, -}: TraitEditorProps) { - const availableTraitNames = [ - 'aws_role_arns', - 'azure_identities', - 'db_names', - 'db_roles', - 'db_users', - 'gcp_service_accounts', - 'host_user_gid', - 'host_user_uid', - 'kubernetes_groups', - 'kubernetes_users', - 'logins', - 'windows_logins', - ]; - - useEffect(() => { - let newTrait = []; - for (let trait in allTraits) { - if (!allTraits[trait][0]) { - continue; - } - if (allTraits[trait].length > 0) { - newTrait.push({ - trait: { value: trait, label: trait }, - traitValues: allTraits[trait].map(t => ({ - value: t, - label: t, - })), - }); - } - } - - setConfiguredTraits(newTrait); - }, [allTraits]); - - type InputOption = { - labelField: 'trait' | 'traitValues'; - option: Option | Option[]; - index: number; - }; - - function handleInputChange(i: InputOption) { - // validator.reset() - const newTraits = [...configuredTraits]; - if (i.labelField === 'traitValues') { - let traitValue: Option[] = i.option as Option[]; - - newTraits[i.index] = { - ...newTraits[i.index], - [i.labelField]: [...traitValue], - }; - setConfiguredTraits(newTraits); - } else { - let traitName: Option = i.option as Option; - - newTraits[i.index] = { - ...newTraits[i.index], - [i.labelField]: traitName, - }; - setConfiguredTraits(newTraits); - } - } - - function addTrait() { - const newTraits = [...configuredTraits]; - newTraits.push({ - trait: { value: '', label: 'Select or type new trait name and enter' }, - traitValues: [], - }); - setConfiguredTraits(newTraits); - } - - function removeTrait(index: number) { - const newTraits = [...configuredTraits]; - newTraits.splice(index, 1); - setConfiguredTraits(newTraits); - } - - const addLabelText = - configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; - - const requireNoDuplicateTraits = (enteredTrait: Option) => () => { - let k = configuredTraits.map(trait => trait.trait.value.toLowerCase()); - let occurance = 0; - for (let t in k) { - if (k[t] === enteredTrait.value.toLowerCase()) { - occurance++; - } - } - if (occurance > 1) { - return { valid: false, message: 'Trait key should be unique for a user' }; - } - return { valid: true }; - }; - - return ( - - User Traits - - - {configuredTraits.map(({ trait, traitValues }, index) => { - return ( - - - - ({ - value: r, - label: r, - }))} - placeholder="Select or type new trait name" - autoFocus - isSearchable - value={trait} - label="Key" - rule={requiredAll( - requiredField('Trait key is required'), - requireNoDuplicateTraits - )} - onChange={e => { - handleInputChange({ - option: e as Option, - labelField: 'trait', - index: index, - }); - }} - createOptionPosition="last" - /> - - - props.theme.colors.levels.surface}; - `} - placeholder="Type a new trait value and enter" - defaultValue={traitValues.map(r => ({ - value: r, - label: r, - }))} - label="Value" - isMulti - isSearchable - isClearable={false} - value={traitValues} - rule={requiredField('Trait value cannot be empty')} - onChange={e => { - handleInputChange({ - option: e as Option, - labelField: 'traitValues', - index: index, - }); - }} - isDisabled={false} - formatCreateLabel={(i: string) => - 'Trait value: ' + `"${i}"` - } - /> - - removeTrait(index)} - css={` - &:disabled { - opacity: 0.65; - pointer-events: none; - } - `} - disabled={false} - > - - - - - ); - })} - - - - - - {addLabelText} - - - - ); -} diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 06e164b68f7b2..a9851d5807234 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -22,7 +22,7 @@ import { Option } from 'shared/components/Select'; import { ResetToken, User } from 'teleport/services/user'; -export type TraitEditor = { trait: Option; traitValues: Option[] }; +import type { TraitsOption } from './TraitsEditor'; export default function useUserDialog(props: Props) { const { attempt, setAttempt } = useAttemptNext(''); @@ -34,7 +34,7 @@ export default function useUserDialog(props: Props) { label: r, })) ); - const [configuredTraits, setConfiguredTraits] = useState([]); + const [configuredTraits, setConfiguredTraits] = useState([]); function onChangeName(name = '') { setName(name); From 6dde19bb60c8fd85ed5434bb555c83534539150c Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 12:18:03 -0400 Subject: [PATCH 12/29] fix edit story --- e | 2 +- .../teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/e b/e index 9a1db8260d79d..39485b1a3b09a 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit 9a1db8260d79d29d39994d1456b2828d74d25554 +Subproject commit 39485b1a3b09a9cecdc77e512507942c12d92a26 diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx index dbdb528ec532f..b8be9d482df29 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import React from 'react'; +import React, { useState } from 'react'; import { AllUserTraits } from 'teleport/services/user'; @@ -42,7 +42,8 @@ export const Create = () => { }; export const Edit = () => { - return ; + const [configuredTraits, setConfiguredTraits] = useState([]) + return ; }; export const Processing = () => { From 58d6a7943f7c07bce0cdf3e0daac26f1d5a3d498 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 13:31:15 -0400 Subject: [PATCH 13/29] update /users API to handle allTraits --- e | 2 +- lib/web/users.go | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/e b/e index 39485b1a3b09a..20614eff95d58 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit 39485b1a3b09a9cecdc77e512507942c12d92a26 +Subproject commit 20614eff95d58662d91a8f1e6c727727ad4014f9 diff --git a/lib/web/users.go b/lib/web/users.go index cada9a3996c97..02f2ebb6aae5d 100644 --- a/lib/web/users.go +++ b/lib/web/users.go @@ -169,7 +169,13 @@ func updateUser(r *http.Request, m userAPIGetter) (*ui.User, error) { user.SetRoles(req.Roles) - updateUserTraits(req, user) + // we do not want situation where one trait set (allTraits) + // overrides another (traits). + if len(req.AllTraits) > 0 { + user.SetTraits(req.AllTraits) + } else { + updateUserTraits(req, user) + } updated, err := m.UpdateUser(r.Context(), user) if err != nil { @@ -305,9 +311,10 @@ type userTraits struct { // - if the value is an empty array we remove every element from the trait // - otherwise, we replace the list for that trait type saveUserRequest struct { - Name string `json:"name"` - Roles []string `json:"roles"` - Traits userTraits `json:"traits"` + Name string `json:"name"` + Roles []string `json:"roles"` + Traits userTraits `json:"traits"` + AllTraits map[string][]string `json:"allTraits"` } func (r *saveUserRequest) checkAndSetDefaults() error { From b015d0330d70156e6ca89a1f0043a727f58efb8f Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 14:02:17 -0400 Subject: [PATCH 14/29] cleanup --- e | 2 +- .../Users/UserAddEdit/TraitsEditor.test.tsx | 4 ++ .../src/Users/UserAddEdit/TraitsEditor.tsx | 48 +++++++++---------- .../Users/UserAddEdit/UserAddEdit.story.tsx | 13 +++-- .../src/Users/UserAddEdit/UserAddEdit.tsx | 3 +- .../src/Users/UserAddEdit/useDialog.tsx | 11 ++--- 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/e b/e index 20614eff95d58..9a1db8260d79d 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit 20614eff95d58662d91a8f1e6c727727ad4014f9 +Subproject commit 9a1db8260d79d29d39994d1456b2828d74d25554 diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx index cce729f26b0e3..82ce9af9b3dc7 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -24,6 +24,7 @@ describe('Render traits correctly', () => { const { rerender } = render( @@ -39,6 +40,7 @@ describe('Render traits correctly', () => { @@ -59,6 +61,7 @@ describe('Render traits correctly', () => { @@ -77,6 +80,7 @@ describe('Render traits correctly', () => { diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index ba04ec4299f51..934eaa0b490db 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -22,6 +22,7 @@ import { Add, Trash } from 'design/Icon'; import { FieldSelectCreatable } from 'shared/components/FieldSelect'; import { Option } from 'shared/components/Select'; import { requiredField, requiredAll } from 'shared/components/Validation/rules'; +import { Attempt } from 'shared/hooks/useAttemptNext'; import { AllUserTraits } from 'teleport/services/user'; @@ -42,26 +43,25 @@ const availableTraitNames = [ export function TraitsEditor({ allTraits, + attempt, configuredTraits, setConfiguredTraits, }: TraitEditorProps) { useEffect(() => { - let newTrait = traitsToTraitsOption(allTraits); - - setConfiguredTraits(newTrait); + const newTraits = traitsToTraitsOption(allTraits); + setConfiguredTraits(newTraits); }, [allTraits]); type InputOption = { - labelField: 'trait' | 'traitValues'; + labelField: 'traitKey' | 'traitValues'; option: Option | Option[]; index: number; }; function handleInputChange(i: InputOption) { - const newTraits = [...configuredTraits]; + let newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { let traitValue: Option[] = i.option as Option[]; - newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: [...traitValue], @@ -69,7 +69,6 @@ export function TraitsEditor({ setConfiguredTraits(newTraits); } else { let traitName: Option = i.option as Option; - newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitName, @@ -79,13 +78,13 @@ export function TraitsEditor({ } function addTrait() { - const newTraits = [...configuredTraits]; + let newTraits = [...configuredTraits]; newTraits.push(emptyTrait); setConfiguredTraits(newTraits); } function removeTrait(index: number) { - const newTraits = [...configuredTraits]; + let newTraits = [...configuredTraits]; newTraits.splice(index, 1); setConfiguredTraits(newTraits); } @@ -94,13 +93,13 @@ export function TraitsEditor({ configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; const requireNoDuplicateTraits = (enteredTrait: Option) => () => { - let k = configuredTraits.map(trait => trait.trait.value.toLowerCase()); + let k = configuredTraits.map(trait => trait.traitKey.value.toLowerCase()); let occurance = 0; - for (let t in k) { - if (k[t] === enteredTrait.value.toLowerCase()) { + k.forEach(key => { + if (key === enteredTrait.value.toLowerCase()) { occurance++; } - } + }) if (occurance > 1) { return { valid: false, message: 'Trait key should be unique for a user' }; } @@ -110,9 +109,8 @@ export function TraitsEditor({ return ( User Traits - - {configuredTraits.map(({ trait, traitValues }, index) => { + {configuredTraits.map(({ traitKey, traitValues }, index) => { return ( @@ -126,7 +124,7 @@ export function TraitsEditor({ placeholder="Select or type new trait name and enter" autoFocus isSearchable - value={trait} + value={traitKey} label="Key" rule={requiredAll( requiredField('Trait key is required'), @@ -134,12 +132,13 @@ export function TraitsEditor({ )} onChange={e => { handleInputChange({ - option: e as Option, - labelField: 'trait', + option: e, + labelField: 'traitKey', index: index, }); }} createOptionPosition="last" + isDisabled={attempt.status === 'processing'} /> @@ -163,15 +162,15 @@ export function TraitsEditor({ rule={requiredField('Trait value cannot be empty')} onChange={e => { handleInputChange({ - option: e as Option, + option: e, labelField: 'traitValues', index: index, }); }} - isDisabled={false} formatCreateLabel={(i: string) => 'Trait value: ' + `"${i}"` } + isDisabled={attempt.status === 'processing'} /> @@ -210,7 +209,7 @@ export function TraitsEditor({ pointer-events: none; } `} - disabled={false} + disabled={attempt.status === 'processing'} > >; configuredTraits: TraitsOption[]; + attempt: Attempt; }; diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx index b8be9d482df29..e47b534c07cae 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.story.tsx @@ -42,8 +42,15 @@ export const Create = () => { }; export const Edit = () => { - const [configuredTraits, setConfiguredTraits] = useState([]) - return ; + const [configuredTraits, setConfiguredTraits] = useState([]); + return ( + + ); }; export const Processing = () => { @@ -80,7 +87,7 @@ const props = { allTraits: { ['logins']: ['root', 'ubuntu'] } as AllUserTraits, configuredTraits: [ { - trait: { value: 'logins', label: 'logins' }, + traitKey: { value: 'logins', label: 'logins' }, traitValues: [{ value: 'root', label: 'root' }], }, ] as TraitsOption[], diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index b0e8fc303c184..e3fae53fb0d4e 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -66,8 +66,6 @@ export function UserAddEdit(props: ReturnType) { return; } - console.log('returning without save'); - return; onSave(); } @@ -122,6 +120,7 @@ export function UserAddEdit(props: ReturnType) { /> diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index a9851d5807234..e4eade874538c 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -45,12 +45,11 @@ export default function useUserDialog(props: Props) { } function onSave() { - const traitsToSave = {}; - for (const trait of configuredTraits) { - if (trait.trait.value === '' || trait.traitValues.length === 0) { - continue; - } - traitsToSave[trait.trait.value] = trait.traitValues.map(t => t.value); + let traitsToSave = {}; + for (const traitKV of configuredTraits) { + traitsToSave[traitKV.traitKey.value] = traitKV.traitValues.map( + t => t.value + ); } const u = { name, From ca0520e2465f798b4b63355a7f18afa0abdbeeef Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 14:14:08 -0400 Subject: [PATCH 15/29] prettier format --- web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index 934eaa0b490db..166ce75566944 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -99,7 +99,7 @@ export function TraitsEditor({ if (key === enteredTrait.value.toLowerCase()) { occurance++; } - }) + }); if (occurance > 1) { return { valid: false, message: 'Trait key should be unique for a user' }; } From f5d24c83db7edd7fd9e1ed0b83d4b2c39b9d1134 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 14:27:51 -0400 Subject: [PATCH 16/29] move validation rule out from TraitsEditor component --- .../src/Users/UserAddEdit/TraitsEditor.tsx | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index 166ce75566944..87a19828771b2 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -52,12 +52,6 @@ export function TraitsEditor({ setConfiguredTraits(newTraits); }, [allTraits]); - type InputOption = { - labelField: 'traitKey' | 'traitValues'; - option: Option | Option[]; - index: number; - }; - function handleInputChange(i: InputOption) { let newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { @@ -92,20 +86,6 @@ export function TraitsEditor({ const addLabelText = configuredTraits.length > 0 ? 'Add another user trait' : 'Add user trait'; - const requireNoDuplicateTraits = (enteredTrait: Option) => () => { - let k = configuredTraits.map(trait => trait.traitKey.value.toLowerCase()); - let occurance = 0; - k.forEach(key => { - if (key === enteredTrait.value.toLowerCase()) { - occurance++; - } - }); - if (occurance > 1) { - return { valid: false, message: 'Trait key should be unique for a user' }; - } - return { valid: true }; - }; - return ( User Traits @@ -128,7 +108,7 @@ export function TraitsEditor({ label="Key" rule={requiredAll( requiredField('Trait key is required'), - requireNoDuplicateTraits + requireNoDuplicateTraits(configuredTraits) )} onChange={e => { handleInputChange({ @@ -225,6 +205,27 @@ export function TraitsEditor({ ); } +type InputOption = { + labelField: 'traitKey' | 'traitValues'; + option: Option | Option[]; + index: number; +}; + +const requireNoDuplicateTraits = + (configuredTraits: TraitsOption[]) => (enteredTrait: Option) => () => { + let k = configuredTraits.map(trait => trait.traitKey.value.toLowerCase()); + let occurance = 0; + k.forEach(key => { + if (key === enteredTrait.value.toLowerCase()) { + occurance++; + } + }); + if (occurance > 1) { + return { valid: false, message: 'Trait key should be unique for a user' }; + } + return { valid: true }; + }; + export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { let newTrait = []; for (let trait in allTraits) { @@ -233,7 +234,7 @@ export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { } if (allTraits[trait].length > 0) { newTrait.push({ - trait: { value: trait, label: trait }, + traitKey: { value: trait, label: trait }, traitValues: allTraits[trait].map(t => ({ value: t, label: t, From 5ac8f303e1820770628628f65168566a1c9a017c Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Mon, 10 Jun 2024 17:54:45 -0400 Subject: [PATCH 17/29] remove requiredAll rule wrapper --- .../shared/components/Validation/rules.ts | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/web/packages/shared/components/Validation/rules.ts b/web/packages/shared/components/Validation/rules.ts index 456782f10955b..02cf1e844385b 100644 --- a/web/packages/shared/components/Validation/rules.ts +++ b/web/packages/shared/components/Validation/rules.ts @@ -194,34 +194,6 @@ const requiredEmailLike: Rule = email => () => { }; }; -/** - * A rule function that combines multiple inner rule functions. All rules must - * return `valid`, otherwise it returns a comma separated string containing all - * invalid rule messages. - * @param rules a list of rule functions to apply - * @returns a rule function that ANDs all input rules - */ -const requiredAll = - (...rules: Rule[]): Rule => - (value: T) => - () => { - let messages = []; - for (let r of rules) { - let result = r(value)(); - if (!result.valid) { - messages.push(result.message); - } - } - - if (messages.length > 0) { - return { - valid: false, - message: messages.join('. '), - }; - } - return { valid: true }; - }; - export { requiredToken, requiredPassword, @@ -229,6 +201,5 @@ export { requiredField, requiredRoleArn, requiredIamRoleName, - requiredEmailLike, - requiredAll, + requiredEmailLike }; From 0d6b589bcda5f4c5525ffba6964a281b30d7025b Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Tue, 11 Jun 2024 15:10:15 -0400 Subject: [PATCH 18/29] rename trait vars and add api unit test: - rename traits to traitsPreset - add unit test for allTraits. Update existing checkAndSetDefault test to use table - add comment to user types to differentiate traits and allTraits --- lib/web/users.go | 81 +++++--- lib/web/users_test.go | 187 ++++++++++++++---- .../teleport/src/services/user/types.ts | 10 +- .../teleport/src/services/user/user.ts | 14 ++ 4 files changed, 216 insertions(+), 76 deletions(-) diff --git a/lib/web/users.go b/lib/web/users.go index 02f2ebb6aae5d..7f6c60a2689ff 100644 --- a/lib/web/users.go +++ b/lib/web/users.go @@ -104,7 +104,16 @@ func createUser(r *http.Request, m userAPIGetter, createdBy string) (*ui.User, e user.SetRoles(req.Roles) - updateUserTraits(req, user) + // checkAndSetDefaults makes sure either TraitsPreset + // or AllTraits field to be populated. Since empty + // AllTraits is also used to delete all user traits, + // we explicitly check if TraitsPreset is empty so + // to prevent traits deletion. + if req.TraitsPreset == (traitsPreset{}) { + user.SetTraits(req.AllTraits) + } else { + updateUserTraitsPreset(req, user) + } user.SetCreatedBy(types.CreatedBy{ User: types.UserRef{Name: createdBy}, @@ -119,30 +128,30 @@ func createUser(r *http.Request, m userAPIGetter, createdBy string) (*ui.User, e return ui.NewUser(created) } -// updateUserTraits receives a saveUserRequest and updates the user traits accordingly -// It only updates the traits that have a non-nil value in saveUserRequest -// This allows the partial update of the properties -func updateUserTraits(req *saveUserRequest, user types.User) { - if req.Traits.Logins != nil { - user.SetLogins(*req.Traits.Logins) +// updateUserTraitsPreset receives a saveUserRequest and updates the user traits +// accordingly. It only updates the traits that have a non-nil value in +// saveUserRequest. This allows the partial update of the properties +func updateUserTraitsPreset(req *saveUserRequest, user types.User) { + if req.TraitsPreset.Logins != nil { + user.SetLogins(*req.TraitsPreset.Logins) } - if req.Traits.DatabaseUsers != nil { - user.SetDatabaseUsers(*req.Traits.DatabaseUsers) + if req.TraitsPreset.DatabaseUsers != nil { + user.SetDatabaseUsers(*req.TraitsPreset.DatabaseUsers) } - if req.Traits.DatabaseNames != nil { - user.SetDatabaseNames(*req.Traits.DatabaseNames) + if req.TraitsPreset.DatabaseNames != nil { + user.SetDatabaseNames(*req.TraitsPreset.DatabaseNames) } - if req.Traits.KubeUsers != nil { - user.SetKubeUsers(*req.Traits.KubeUsers) + if req.TraitsPreset.KubeUsers != nil { + user.SetKubeUsers(*req.TraitsPreset.KubeUsers) } - if req.Traits.KubeGroups != nil { - user.SetKubeGroups(*req.Traits.KubeGroups) + if req.TraitsPreset.KubeGroups != nil { + user.SetKubeGroups(*req.TraitsPreset.KubeGroups) } - if req.Traits.WindowsLogins != nil { - user.SetWindowsLogins(*req.Traits.WindowsLogins) + if req.TraitsPreset.WindowsLogins != nil { + user.SetWindowsLogins(*req.TraitsPreset.WindowsLogins) } - if req.Traits.AWSRoleARNs != nil { - user.SetAWSRoleARNs(*req.Traits.AWSRoleARNs) + if req.TraitsPreset.AWSRoleARNs != nil { + user.SetAWSRoleARNs(*req.TraitsPreset.AWSRoleARNs) } } @@ -169,12 +178,15 @@ func updateUser(r *http.Request, m userAPIGetter) (*ui.User, error) { user.SetRoles(req.Roles) - // we do not want situation where one trait set (allTraits) - // overrides another (traits). - if len(req.AllTraits) > 0 { + // checkAndSetDefaults makes sure either TraitsPreset + // or AllTraits field to be populated. Since empty + // AllTraits is also used to delete all user traits, + // we explicitly check if TraitsPreset is empty so + // to prevent traits deletion. + if req.TraitsPreset == (traitsPreset{}) { user.SetTraits(req.AllTraits) } else { - updateUserTraits(req, user) + updateUserTraitsPreset(req, user) } updated, err := m.UpdateUser(r.Context(), user) @@ -293,7 +305,8 @@ type userAPIGetter interface { DeleteUser(ctx context.Context, user string) error } -type userTraits struct { +// traitsPreset are user traits that are pre-defined in Teleport +type traitsPreset struct { Logins *[]string `json:"logins,omitempty"` DatabaseUsers *[]string `json:"databaseUsers,omitempty"` DatabaseNames *[]string `json:"databaseNames,omitempty"` @@ -309,11 +322,20 @@ type userTraits struct { // They are optional and respect the following logic: // - if the value is nil, we ignore it // - if the value is an empty array we remove every element from the trait -// - otherwise, we replace the list for that trait +// - otherwise, we replace the list for that trait. +// Use TraitsPreset to selectively update traits. +// Use AllTraits to fully replace existing traits. type saveUserRequest struct { - Name string `json:"name"` - Roles []string `json:"roles"` - Traits userTraits `json:"traits"` + // Name is username. + Name string `json:"name"` + // Roles is slice of user roles assigned to user. + Roles []string `json:"roles"` + // TraitsPreset holds traits that are pre-defined in Teleport. + // Clients may use TraitsPreset to selectively update user traits. + TraitsPreset traitsPreset `json:"traits"` + // AllTraits may hold all the user traits, including traits key defined + // in TraitsPreset and/or new trait key values defined by Teleport admin. + // AllTraits should be used to fully replace and update user traits. AllTraits map[string][]string `json:"allTraits"` } @@ -324,5 +346,8 @@ func (r *saveUserRequest) checkAndSetDefaults() error { if len(r.Roles) == 0 { return trace.BadParameter("missing roles") } + if len(r.AllTraits) != 0 && r.TraitsPreset != (traitsPreset{}) { + return trace.BadParameter("either traits or allTraits must be provided") + } return nil } diff --git a/lib/web/users_test.go b/lib/web/users_test.go index 5cd9beb80f985..929fa54add7bd 100644 --- a/lib/web/users_test.go +++ b/lib/web/users_test.go @@ -34,40 +34,100 @@ import ( ) func TestRequestParameters(t *testing.T) { - r := saveUserRequest{ - Name: "", - Roles: nil, - Traits: userTraits{}, - } - require.True(t, trace.IsBadParameter(r.checkAndSetDefaults())) - - r = saveUserRequest{ - Name: "", - Roles: []string{"testrole"}, - Traits: userTraits{}, + tests := []struct { + name string + username string + role []string + traitsPreset traitsPreset + allTraits map[string][]string + errAssertion require.ErrorAssertionFunc + }{ + { + name: "empty request", + username: "", + role: nil, + traitsPreset: traitsPreset{}, + allTraits: nil, + errAssertion: func(t require.TestingT, err error, i ...interface{}) { + require.ErrorIs(t, err, trace.BadParameter("missing user name")) + }, + }, + { + name: "empty name", + username: "", + role: []string{"testrole"}, + traitsPreset: traitsPreset{}, + allTraits: nil, + errAssertion: func(t require.TestingT, err error, i ...interface{}) { + require.ErrorIs(t, err, trace.BadParameter("missing user name")) + }, + }, + { + name: "empty role", + username: "testuser", + role: nil, + traitsPreset: traitsPreset{}, + allTraits: nil, + errAssertion: func(t require.TestingT, err error, i ...interface{}) { + require.ErrorIs(t, err, trace.BadParameter("missing roles")) + }, + }, + { + name: "both traitsPreset and allTraits", + username: "testuser", + role: []string{"testrole"}, + traitsPreset: traitsPreset{Logins: &[]string{"root"}}, + allTraits: map[string][]string{"logins": {"root"}}, + errAssertion: func(t require.TestingT, err error, i ...interface{}) { + require.ErrorIs(t, err, trace.BadParameter("either traits or allTraits must be provided")) + }, + }, + { + name: "user without traits", + username: "testuser", + role: []string{"testrole"}, + traitsPreset: traitsPreset{}, + allTraits: nil, + errAssertion: require.NoError, + }, + { + name: "user with traitsPreset", + username: "testuser", + role: []string{"testrole"}, + traitsPreset: traitsPreset{Logins: &[]string{"root"}}, + allTraits: map[string][]string{}, + errAssertion: require.NoError, + }, + { + name: "user with allTraits", + username: "testuser", + role: []string{"testrole"}, + traitsPreset: traitsPreset{}, + allTraits: map[string][]string{"logins": {"root"}}, + errAssertion: require.NoError, + }, } - require.True(t, trace.IsBadParameter(r.checkAndSetDefaults())) - r = saveUserRequest{ - Name: "username", - Roles: nil, - Traits: userTraits{}, - } - require.True(t, trace.IsBadParameter(r.checkAndSetDefaults())) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := saveUserRequest{ + Name: test.username, + Roles: test.role, + TraitsPreset: test.traitsPreset, + AllTraits: test.allTraits, + } - r = saveUserRequest{ - Name: "username", - Roles: []string{"testrole"}, - Traits: userTraits{}, + err := r.checkAndSetDefaults() + test.errAssertion(t, err) + }) } - require.NoError(t, r.checkAndSetDefaults()) } func TestCRUDs(t *testing.T) { u := saveUserRequest{ - Name: "testname", - Roles: []string{"testrole"}, - Traits: userTraits{}, + Name: "testname", + Roles: []string{"testrole"}, + TraitsPreset: traitsPreset{}, } m := &mockedUserAPIGetter{} @@ -120,7 +180,7 @@ func TestCRUDs(t *testing.T) { require.NoError(t, err) } -func TestUpdateUser_setTraits(t *testing.T) { +func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { defaultRoles := []string{"role1"} defaultLogins := []string{"login1"} tests := []struct { @@ -131,9 +191,9 @@ func TestUpdateUser_setTraits(t *testing.T) { { name: "Logins", updateReq: saveUserRequest{ - Name: "setlogins", - Roles: defaultRoles, - Traits: userTraits{Logins: &[]string{"login1", "login2"}}, + Name: "setlogins", + Roles: defaultRoles, + TraitsPreset: traitsPreset{Logins: &[]string{"login1", "login2"}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {"login1", "login2"}, @@ -144,7 +204,7 @@ func TestUpdateUser_setTraits(t *testing.T) { updateReq: saveUserRequest{ Name: "setdb", Roles: defaultRoles, - Traits: userTraits{ + TraitsPreset: traitsPreset{ Logins: &defaultLogins, DatabaseUsers: &[]string{"dbuser1", "dbuser2"}, DatabaseNames: &[]string{"dbname1", "dbname2"}, @@ -161,7 +221,7 @@ func TestUpdateUser_setTraits(t *testing.T) { updateReq: saveUserRequest{ Name: "setkube", Roles: defaultRoles, - Traits: userTraits{ + TraitsPreset: traitsPreset{ Logins: &defaultLogins, KubeUsers: &[]string{"kubeuser1", "kubeuser2"}, KubeGroups: &[]string{"kubegroup1", "kubegroup2"}, @@ -178,7 +238,7 @@ func TestUpdateUser_setTraits(t *testing.T) { updateReq: saveUserRequest{ Name: "setwindowslogins", Roles: defaultRoles, - Traits: userTraits{ + TraitsPreset: traitsPreset{ Logins: &defaultLogins, WindowsLogins: &[]string{"login1", "login2"}, }, @@ -193,7 +253,7 @@ func TestUpdateUser_setTraits(t *testing.T) { updateReq: saveUserRequest{ Name: "setawsrolearns", Roles: defaultRoles, - Traits: userTraits{ + TraitsPreset: traitsPreset{ Logins: &defaultLogins, AWSRoleARNs: &[]string{"arn1", "arn2"}, }, @@ -206,9 +266,9 @@ func TestUpdateUser_setTraits(t *testing.T) { { name: "Deduplicates", updateReq: saveUserRequest{ - Name: "deduplicates", - Roles: defaultRoles, - Traits: userTraits{Logins: &[]string{"login1", "login2", "login1"}}, + Name: "deduplicates", + Roles: defaultRoles, + TraitsPreset: traitsPreset{Logins: &[]string{"login1", "login2", "login1"}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {"login1", "login2"}, @@ -217,9 +277,9 @@ func TestUpdateUser_setTraits(t *testing.T) { { name: "RemovesAll", updateReq: saveUserRequest{ - Name: "removesall", - Roles: defaultRoles, - Traits: userTraits{Logins: &[]string{}}, + Name: "removesall", + Roles: defaultRoles, + TraitsPreset: traitsPreset{Logins: &[]string{}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {}, @@ -268,6 +328,47 @@ func TestUpdateUser_setTraits(t *testing.T) { } } +func TestUpdateUser_setTraitsWithAllTraits(t *testing.T) { + defaultRoles := []string{"role1"} + + // create user + user, err := types.NewUser("alice") + require.NoError(t, err) + user.SetRoles(defaultRoles) + + m := &mockedUserAPIGetter{} + m.mockGetUser = func(ctx context.Context, name string, withSecrets bool) (types.User, error) { + return user, nil + } + m.mockUpdateUser = func(ctx context.Context, user types.User) (types.User, error) { + return user, nil + } + + // update user with AllTraits + allTraitsWithValue := saveUserRequest{ + Name: "setlogins", + Roles: defaultRoles, + AllTraits: map[string][]string{"logins": {"root", "admin"}}, + } + _, err = updateUser(newRequest(t, allTraitsWithValue), m) + require.NoError(t, err) + require.Equal(t, allTraitsWithValue.AllTraits, user.GetTraits()) + + // verify other fields dont't change + require.ElementsMatch(t, user.GetRoles(), defaultRoles) + + // update user with empty AllTraits + emptyAllTraits := saveUserRequest{ + Name: "setlogins", + Roles: defaultRoles, + AllTraits: map[string][]string{}, + } + _, err = updateUser(newRequest(t, emptyAllTraits), m) + require.NoError(t, err) + // empty AllTraits field should delete existing traits + require.Equal(t, emptyAllTraits.AllTraits, user.GetTraits()) +} + func TestCRUDErrors(t *testing.T) { m := &mockedUserAPIGetter{} m.mockCreateUser = func(ctx context.Context, user types.User) (types.User, error) { @@ -291,9 +392,9 @@ func TestCRUDErrors(t *testing.T) { } u := saveUserRequest{ - Name: "testname", - Roles: []string{"testrole"}, - Traits: userTraits{Logins: nil}, + Name: "testname", + Roles: []string{"testrole"}, + TraitsPreset: traitsPreset{Logins: nil}, } // update errors diff --git a/web/packages/teleport/src/services/user/types.ts b/web/packages/teleport/src/services/user/types.ts index 9c5a45537d58f..9ef3736dd27f1 100644 --- a/web/packages/teleport/src/services/user/types.ts +++ b/web/packages/teleport/src/services/user/types.ts @@ -125,12 +125,12 @@ export interface User { isLocal?: boolean; // isBot is true if the user is a Bot User. isBot?: boolean; - // traits existed before field "externalTraits" - // and returns only "specific" traits. + // traits are preset traits defined in Teleport, such as + // logins, db_role etc. These traits are defiend in UserTraits interface. traits?: UserTraits; - // externalTraits came after field "traits" - // and contains ALL the traits defined for - // this user. + // allTraits contains both preset traits, as well as externalTraits + // such as those created by external IdP attributes to roles mapping + // or new values as set by Teleport admin. allTraits?: AllUserTraits; } diff --git a/web/packages/teleport/src/services/user/user.ts b/web/packages/teleport/src/services/user/user.ts index c0fa25c888073..19da6a8075a1c 100644 --- a/web/packages/teleport/src/services/user/user.ts +++ b/web/packages/teleport/src/services/user/user.ts @@ -58,10 +58,24 @@ const service = { return api.get(cfg.getUsersUrl()).then(makeUsers); }, + /** + * Update user. + * use allTraits to create new or replace entire user traits. + * use traits to selectively add/update user traits. + * @param user + * @returns user + */ updateUser(user: User) { return api.put(cfg.getUsersUrl(), user).then(makeUser); }, + /** + * Create user. + * use allTraits to create new or replace entire user traits. + * use traits to selectively add/update user traits. + * @param user + * @returns user + */ createUser(user: User, webauthnResponse?: WebauthnAssertionResponse) { return api .post(cfg.getUsersUrl(), user, null, webauthnResponse) From 2f133574f196167405ff31ad88d4a59cdf625185 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Tue, 11 Jun 2024 16:02:42 -0400 Subject: [PATCH 19/29] fix typo's, add js comments --- .../Users/UserAddEdit/TraitsEditor.test.tsx | 10 +++----- .../src/Users/UserAddEdit/TraitsEditor.tsx | 23 ++++++++++++++----- .../teleport/src/services/user/user.ts | 2 +- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx index 82ce9af9b3dc7..06f374c93bd6d 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -16,9 +16,8 @@ describe('Render traits correctly', () => { db_names: ['postgres', 'aurora'], } as AllUserTraits; - test('Avalable traits are rendered', async () => { - let configuredTraits = [] as TraitsOption[]; - + test('Available traits are rendered', async () => { + const configuredTraits = [] as TraitsOption[]; const setConfiguredTraits = jest.fn(); const { rerender } = render( @@ -31,7 +30,6 @@ describe('Render traits correctly', () => { ); expect(screen.getByText('User Traits')).toBeInTheDocument(); - expect(setConfiguredTraits).toHaveBeenLastCalledWith( traitsToTraitsOption(userTraits) ); @@ -52,9 +50,7 @@ describe('Render traits correctly', () => { test('Add and remove Trait', async () => { const userTraits = {} as AllUserTraits; - - let configuredTraits = [] as TraitsOption[]; - + const configuredTraits = [] as TraitsOption[]; const setConfiguredTraits = jest.fn(); const { rerender } = render( diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index 87a19828771b2..740e55c4ee421 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -26,7 +26,11 @@ import { Attempt } from 'shared/hooks/useAttemptNext'; import { AllUserTraits } from 'teleport/services/user'; -const availableTraitNames = [ +/** + * traitsPreset is a list of system defined traits in Teleport. + * The list is used to populate traits key option. + */ +const traitsPreset = [ 'aws_role_arns', 'azure_identities', 'db_names', @@ -41,6 +45,13 @@ const availableTraitNames = [ 'windows_logins', ]; +/** + * TraitsEditor supports add, edit or remove traits functionality. + * @param allTraits all traits pre-configured for user. + * @param attempt attempt is Attempt status. + * @param configuredTraits holds traits configured for user in current editor. + * @param setConfiguredTraits sets user traits in current editor. + */ export function TraitsEditor({ allTraits, attempt, @@ -71,13 +82,13 @@ export function TraitsEditor({ } } - function addTrait() { + function addNewTraitPair() { let newTraits = [...configuredTraits]; newTraits.push(emptyTrait); setConfiguredTraits(newTraits); } - function removeTrait(index: number) { + function removeTraitPair(index: number) { let newTraits = [...configuredTraits]; newTraits.splice(index, 1); setConfiguredTraits(newTraits); @@ -97,7 +108,7 @@ export function TraitsEditor({ ({ + options={traitsPreset.map(r => ({ value: r, label: r, }))} @@ -159,7 +170,7 @@ export function TraitsEditor({ size={1} title="Remove Trait" aria-label="Remove Trait" - onClick={() => removeTrait(index)} + onClick={() => removeTraitPair(index)} css={` &:disabled { opacity: 0.65; @@ -178,7 +189,7 @@ export function TraitsEditor({ Date: Tue, 11 Jun 2024 20:20:21 -0400 Subject: [PATCH 20/29] use pointer to traitsPreset so empty struct can be compared with nil --- lib/web/users.go | 8 ++++---- lib/web/users_test.go | 34 +++++++++++++++++----------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/web/users.go b/lib/web/users.go index 7f6c60a2689ff..8594b501b0ac6 100644 --- a/lib/web/users.go +++ b/lib/web/users.go @@ -109,7 +109,7 @@ func createUser(r *http.Request, m userAPIGetter, createdBy string) (*ui.User, e // AllTraits is also used to delete all user traits, // we explicitly check if TraitsPreset is empty so // to prevent traits deletion. - if req.TraitsPreset == (traitsPreset{}) { + if req.TraitsPreset == nil { user.SetTraits(req.AllTraits) } else { updateUserTraitsPreset(req, user) @@ -183,7 +183,7 @@ func updateUser(r *http.Request, m userAPIGetter) (*ui.User, error) { // AllTraits is also used to delete all user traits, // we explicitly check if TraitsPreset is empty so // to prevent traits deletion. - if req.TraitsPreset == (traitsPreset{}) { + if req.TraitsPreset == nil { user.SetTraits(req.AllTraits) } else { updateUserTraitsPreset(req, user) @@ -332,7 +332,7 @@ type saveUserRequest struct { Roles []string `json:"roles"` // TraitsPreset holds traits that are pre-defined in Teleport. // Clients may use TraitsPreset to selectively update user traits. - TraitsPreset traitsPreset `json:"traits"` + TraitsPreset *traitsPreset `json:"traits"` // AllTraits may hold all the user traits, including traits key defined // in TraitsPreset and/or new trait key values defined by Teleport admin. // AllTraits should be used to fully replace and update user traits. @@ -346,7 +346,7 @@ func (r *saveUserRequest) checkAndSetDefaults() error { if len(r.Roles) == 0 { return trace.BadParameter("missing roles") } - if len(r.AllTraits) != 0 && r.TraitsPreset != (traitsPreset{}) { + if len(r.AllTraits) != 0 && r.TraitsPreset != nil { return trace.BadParameter("either traits or allTraits must be provided") } return nil diff --git a/lib/web/users_test.go b/lib/web/users_test.go index 929fa54add7bd..9d8d729f0544e 100644 --- a/lib/web/users_test.go +++ b/lib/web/users_test.go @@ -38,7 +38,7 @@ func TestRequestParameters(t *testing.T) { name string username string role []string - traitsPreset traitsPreset + traitsPreset *traitsPreset allTraits map[string][]string errAssertion require.ErrorAssertionFunc }{ @@ -46,7 +46,7 @@ func TestRequestParameters(t *testing.T) { name: "empty request", username: "", role: nil, - traitsPreset: traitsPreset{}, + traitsPreset: nil, allTraits: nil, errAssertion: func(t require.TestingT, err error, i ...interface{}) { require.ErrorIs(t, err, trace.BadParameter("missing user name")) @@ -56,7 +56,7 @@ func TestRequestParameters(t *testing.T) { name: "empty name", username: "", role: []string{"testrole"}, - traitsPreset: traitsPreset{}, + traitsPreset: nil, allTraits: nil, errAssertion: func(t require.TestingT, err error, i ...interface{}) { require.ErrorIs(t, err, trace.BadParameter("missing user name")) @@ -66,7 +66,7 @@ func TestRequestParameters(t *testing.T) { name: "empty role", username: "testuser", role: nil, - traitsPreset: traitsPreset{}, + traitsPreset: nil, allTraits: nil, errAssertion: func(t require.TestingT, err error, i ...interface{}) { require.ErrorIs(t, err, trace.BadParameter("missing roles")) @@ -76,7 +76,7 @@ func TestRequestParameters(t *testing.T) { name: "both traitsPreset and allTraits", username: "testuser", role: []string{"testrole"}, - traitsPreset: traitsPreset{Logins: &[]string{"root"}}, + traitsPreset: &traitsPreset{Logins: &[]string{"root"}}, allTraits: map[string][]string{"logins": {"root"}}, errAssertion: func(t require.TestingT, err error, i ...interface{}) { require.ErrorIs(t, err, trace.BadParameter("either traits or allTraits must be provided")) @@ -86,7 +86,7 @@ func TestRequestParameters(t *testing.T) { name: "user without traits", username: "testuser", role: []string{"testrole"}, - traitsPreset: traitsPreset{}, + traitsPreset: nil, allTraits: nil, errAssertion: require.NoError, }, @@ -94,7 +94,7 @@ func TestRequestParameters(t *testing.T) { name: "user with traitsPreset", username: "testuser", role: []string{"testrole"}, - traitsPreset: traitsPreset{Logins: &[]string{"root"}}, + traitsPreset: &traitsPreset{Logins: &[]string{"root"}}, allTraits: map[string][]string{}, errAssertion: require.NoError, }, @@ -102,7 +102,7 @@ func TestRequestParameters(t *testing.T) { name: "user with allTraits", username: "testuser", role: []string{"testrole"}, - traitsPreset: traitsPreset{}, + traitsPreset: nil, allTraits: map[string][]string{"logins": {"root"}}, errAssertion: require.NoError, }, @@ -127,7 +127,7 @@ func TestCRUDs(t *testing.T) { u := saveUserRequest{ Name: "testname", Roles: []string{"testrole"}, - TraitsPreset: traitsPreset{}, + TraitsPreset: nil, } m := &mockedUserAPIGetter{} @@ -193,7 +193,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "setlogins", Roles: defaultRoles, - TraitsPreset: traitsPreset{Logins: &[]string{"login1", "login2"}}, + TraitsPreset: &traitsPreset{Logins: &[]string{"login1", "login2"}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {"login1", "login2"}, @@ -204,7 +204,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "setdb", Roles: defaultRoles, - TraitsPreset: traitsPreset{ + TraitsPreset: &traitsPreset{ Logins: &defaultLogins, DatabaseUsers: &[]string{"dbuser1", "dbuser2"}, DatabaseNames: &[]string{"dbname1", "dbname2"}, @@ -221,7 +221,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "setkube", Roles: defaultRoles, - TraitsPreset: traitsPreset{ + TraitsPreset: &traitsPreset{ Logins: &defaultLogins, KubeUsers: &[]string{"kubeuser1", "kubeuser2"}, KubeGroups: &[]string{"kubegroup1", "kubegroup2"}, @@ -238,7 +238,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "setwindowslogins", Roles: defaultRoles, - TraitsPreset: traitsPreset{ + TraitsPreset: &traitsPreset{ Logins: &defaultLogins, WindowsLogins: &[]string{"login1", "login2"}, }, @@ -253,7 +253,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "setawsrolearns", Roles: defaultRoles, - TraitsPreset: traitsPreset{ + TraitsPreset: &traitsPreset{ Logins: &defaultLogins, AWSRoleARNs: &[]string{"arn1", "arn2"}, }, @@ -268,7 +268,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "deduplicates", Roles: defaultRoles, - TraitsPreset: traitsPreset{Logins: &[]string{"login1", "login2", "login1"}}, + TraitsPreset: &traitsPreset{Logins: &[]string{"login1", "login2", "login1"}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {"login1", "login2"}, @@ -279,7 +279,7 @@ func TestUpdateUser_updateUserTraitsPreset(t *testing.T) { updateReq: saveUserRequest{ Name: "removesall", Roles: defaultRoles, - TraitsPreset: traitsPreset{Logins: &[]string{}}, + TraitsPreset: &traitsPreset{Logins: &[]string{}}, }, expectedTraits: map[string][]string{ constants.TraitLogins: {}, @@ -394,7 +394,7 @@ func TestCRUDErrors(t *testing.T) { u := saveUserRequest{ Name: "testname", Roles: []string{"testrole"}, - TraitsPreset: traitsPreset{Logins: nil}, + TraitsPreset: &traitsPreset{Logins: nil}, } // update errors From 314783e29151dc5c6c3035f684928298f6bf7000 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 12:20:07 -0400 Subject: [PATCH 21/29] resovle review comments: - remove double type declaration - move traitsToTraitsOption to be used as default value in setConfiguredTraits - label copy update - udpate test --- .../Users/UserAddEdit/TraitsEditor.test.tsx | 34 +++------- .../src/Users/UserAddEdit/TraitsEditor.tsx | 63 ++++++------------- .../src/Users/UserAddEdit/UserAddEdit.tsx | 2 - .../src/Users/UserAddEdit/useDialog.tsx | 25 +++++++- 4 files changed, 52 insertions(+), 72 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx index 06f374c93bd6d..6338a2cdf5249 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -5,58 +5,45 @@ import Validation from 'shared/components/Validation'; import { AllUserTraits } from 'teleport/services/user'; -import { TraitsEditor, traitsToTraitsOption, emptyTrait } from './TraitsEditor'; +import { TraitsEditor, emptyTrait } from './TraitsEditor'; + +import { traitsToTraitsOption } from './useDialog'; import type { TraitsOption } from './TraitsEditor'; describe('Render traits correctly', () => { - const userTraits = { + const userTraits: AllUserTraits = { logins: ['root', 'ubuntu'], db_roles: ['dbadmin', 'postgres'], db_names: ['postgres', 'aurora'], - } as AllUserTraits; + }; test('Available traits are rendered', async () => { - const configuredTraits = [] as TraitsOption[]; + // const configuredTraits: TraitsOption[] = []; const setConfiguredTraits = jest.fn(); - const { rerender } = render( - - ); - - expect(screen.getByText('User Traits')).toBeInTheDocument(); - expect(setConfiguredTraits).toHaveBeenLastCalledWith( - traitsToTraitsOption(userTraits) - ); - - rerender( + render( ); + + expect(screen.getByText('User Traits')).toBeInTheDocument(); expect(screen.getAllByTestId('trait-key')).toHaveLength(3); expect(screen.getAllByTestId('trait-value')).toHaveLength(3); }); test('Add and remove Trait', async () => { - const userTraits = {} as AllUserTraits; - const configuredTraits = [] as TraitsOption[]; + const configuredTraits: TraitsOption[] = []; const setConfiguredTraits = jest.fn(); const { rerender } = render( { rerender( . */ -import React, { useEffect, Dispatch, SetStateAction } from 'react'; +import React, { Dispatch, SetStateAction } from 'react'; import { ButtonBorder, Box, Flex, Text, ButtonIcon } from 'design'; import { Add, Trash } from 'design/Icon'; import { FieldSelectCreatable } from 'shared/components/FieldSelect'; @@ -24,8 +24,6 @@ import { Option } from 'shared/components/Select'; import { requiredField, requiredAll } from 'shared/components/Validation/rules'; import { Attempt } from 'shared/hooks/useAttemptNext'; -import { AllUserTraits } from 'teleport/services/user'; - /** * traitsPreset is a list of system defined traits in Teleport. * The list is used to populate traits key option. @@ -47,33 +45,26 @@ const traitsPreset = [ /** * TraitsEditor supports add, edit or remove traits functionality. - * @param allTraits all traits pre-configured for user. * @param attempt attempt is Attempt status. * @param configuredTraits holds traits configured for user in current editor. * @param setConfiguredTraits sets user traits in current editor. */ export function TraitsEditor({ - allTraits, attempt, configuredTraits, setConfiguredTraits, }: TraitEditorProps) { - useEffect(() => { - const newTraits = traitsToTraitsOption(allTraits); - setConfiguredTraits(newTraits); - }, [allTraits]); - - function handleInputChange(i: InputOption) { + function handleInputChange(i: InputOption | InputOptionArray) { let newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { - let traitValue: Option[] = i.option as Option[]; + let traitValue: Option[] = i.option; newTraits[i.index] = { ...newTraits[i.index], - [i.labelField]: [...traitValue], + [i.labelField]: traitValue ?? [], }; setConfiguredTraits(newTraits); } else { - let traitName: Option = i.option as Option; + let traitName: Option = i.option; newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitName, @@ -83,9 +74,7 @@ export function TraitsEditor({ } function addNewTraitPair() { - let newTraits = [...configuredTraits]; - newTraits.push(emptyTrait); - setConfiguredTraits(newTraits); + setConfiguredTraits([...configuredTraits, emptyTrait]); } function removeTraitPair(index: number) { @@ -112,7 +101,7 @@ export function TraitsEditor({ value: r, label: r, }))} - placeholder="Select or type new trait name and enter" + placeholder="Type a trait name and press enter" autoFocus isSearchable value={traitKey} @@ -123,7 +112,7 @@ export function TraitsEditor({ )} onChange={e => { handleInputChange({ - option: e, + option: e as Option, labelField: 'traitKey', index: index, }); @@ -140,7 +129,7 @@ export function TraitsEditor({ css={` background: ${props => props.theme.colors.levels.surface}; `} - placeholder="Type a new trait value and enter" + placeholder="Type a trait value and press enter" defaultValue={traitValues.map(r => ({ value: r, label: r, @@ -153,7 +142,7 @@ export function TraitsEditor({ rule={requiredField('Trait value cannot be empty')} onChange={e => { handleInputChange({ - option: e, + option: e as Option[], labelField: 'traitValues', index: index, }); @@ -217,8 +206,14 @@ export function TraitsEditor({ } type InputOption = { - labelField: 'traitKey' | 'traitValues'; - option: Option | Option[]; + labelField: 'traitKey'; + option: Option; + index: number; +}; + +type InputOptionArray = { + labelField: 'traitValues'; + option: Option[]; index: number; }; @@ -237,34 +232,14 @@ const requireNoDuplicateTraits = return { valid: true }; }; -export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { - let newTrait = []; - for (let trait in allTraits) { - if (!allTraits[trait][0]) { - continue; - } - if (allTraits[trait].length > 0) { - newTrait.push({ - traitKey: { value: trait, label: trait }, - traitValues: allTraits[trait].map(t => ({ - value: t, - label: t, - })), - }); - } - } - return newTrait; -} - export const emptyTrait = { - traitKey: { value: '', label: 'Select or type new trait name and enter' }, + traitKey: { value: '', label: 'Type a trait name and press enter' }, traitValues: [], }; export type TraitsOption = { traitKey: Option; traitValues: Option[] }; export type TraitEditorProps = { - allTraits: AllUserTraits; setConfiguredTraits: Dispatch>; configuredTraits: TraitsOption[]; attempt: Attempt; diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index e3fae53fb0d4e..e8cf640919461 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -47,7 +47,6 @@ export function UserAddEdit(props: ReturnType) { onClose, fetchRoles, setConfiguredTraits, - allTraits, attempt, name, selectedRoles, @@ -119,7 +118,6 @@ export function UserAddEdit(props: ReturnType) { elevated={true} /> ([]); + const [configuredTraits, setConfiguredTraits] = useState( + traitsToTraitsOption(props.user.allTraits) + ); function onChangeName(name = '') { setName(name); @@ -104,3 +106,22 @@ export type Props = { onCreate(user: User): Promise; onUpdate(user: User): Promise; }; + +export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { + let newTrait = []; + for (let trait in allTraits) { + if (!allTraits[trait][0]) { + continue; + } + if (allTraits[trait].length > 0) { + newTrait.push({ + traitKey: { value: trait, label: trait }, + traitValues: allTraits[trait].map(t => ({ + value: t, + label: t, + })), + }); + } + } + return newTrait; +} From 1a988728d0db234d0b53a32bbb2180e9d560fe0e Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 12:50:58 -0400 Subject: [PATCH 22/29] remove describe --- .../Users/UserAddEdit/TraitsEditor.test.tsx | 92 +++++++++---------- .../src/Users/UserAddEdit/useDialog.tsx | 6 +- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx index 6338a2cdf5249..e101eb354c871 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -11,64 +11,60 @@ import { traitsToTraitsOption } from './useDialog'; import type { TraitsOption } from './TraitsEditor'; -describe('Render traits correctly', () => { +test('Available traits are rendered', async () => { + const setConfiguredTraits = jest.fn(); const userTraits: AllUserTraits = { logins: ['root', 'ubuntu'], db_roles: ['dbadmin', 'postgres'], db_names: ['postgres', 'aurora'], }; - test('Available traits are rendered', async () => { - // const configuredTraits: TraitsOption[] = []; - const setConfiguredTraits = jest.fn(); + render( + + + + ); - render( - - - - ); - - expect(screen.getByText('User Traits')).toBeInTheDocument(); - expect(screen.getAllByTestId('trait-key')).toHaveLength(3); - expect(screen.getAllByTestId('trait-value')).toHaveLength(3); - }); + expect(screen.getByText('User Traits')).toBeInTheDocument(); + expect(screen.getAllByTestId('trait-key')).toHaveLength(3); + expect(screen.getAllByTestId('trait-value')).toHaveLength(3); +}); - test('Add and remove Trait', async () => { - const configuredTraits: TraitsOption[] = []; - const setConfiguredTraits = jest.fn(); +test('Add and remove Trait', async () => { + const configuredTraits: TraitsOption[] = []; + const setConfiguredTraits = jest.fn(); - const { rerender } = render( - - - - ); - expect(screen.queryAllByTestId('trait-key')).toHaveLength(0); + const { rerender } = render( + + + + ); + expect(screen.queryAllByTestId('trait-key')).toHaveLength(0); - const addButtonEl = screen.getByRole('button', { name: /Add user trait/i }); - expect(addButtonEl).toBeInTheDocument(); - fireEvent.click(addButtonEl); + const addButtonEl = screen.getByRole('button', { name: /Add user trait/i }); + expect(addButtonEl).toBeInTheDocument(); + fireEvent.click(addButtonEl); - expect(setConfiguredTraits).toHaveBeenLastCalledWith([emptyTrait]); + expect(setConfiguredTraits).toHaveBeenLastCalledWith([emptyTrait]); - const singleTrait = { logins: ['root', 'ubuntu'] }; - rerender( - - - - ); - fireEvent.click(screen.getByTitle('Remove Trait')); - expect(setConfiguredTraits).toHaveBeenCalledWith([]); - }); + const singleTrait = { logins: ['root', 'ubuntu'] }; + rerender( + + + + ); + fireEvent.click(screen.getByTitle('Remove Trait')); + expect(setConfiguredTraits).toHaveBeenCalledWith([]); }); diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 6ba16ed6d34a2..83cfe6bcb27db 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -110,9 +110,9 @@ export type Props = { export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { let newTrait = []; for (let trait in allTraits) { - if (!allTraits[trait][0]) { - continue; - } + // if (!allTraits[trait][0]) { + // continue; + // } if (allTraits[trait].length > 0) { newTrait.push({ traitKey: { value: trait, label: trait }, From 23e8524c5095bcfe77bf142721a5ce11b4b9e0c5 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 13:16:10 -0400 Subject: [PATCH 23/29] use const for array, bring back commented empty string checker block --- .../src/Users/UserAddEdit/TraitsEditor.tsx | 14 ++++++++------ .../teleport/src/Users/UserAddEdit/useDialog.tsx | 8 ++++---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index b6e50fbce44d5..3f9b3c8215e37 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -55,16 +55,16 @@ export function TraitsEditor({ setConfiguredTraits, }: TraitEditorProps) { function handleInputChange(i: InputOption | InputOptionArray) { - let newTraits = [...configuredTraits]; + const newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { - let traitValue: Option[] = i.option; + const traitValue: Option[] = i.option; newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitValue ?? [], }; setConfiguredTraits(newTraits); } else { - let traitName: Option = i.option; + const traitName: Option = i.option; newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitName, @@ -78,7 +78,7 @@ export function TraitsEditor({ } function removeTraitPair(index: number) { - let newTraits = [...configuredTraits]; + const newTraits = [...configuredTraits]; newTraits.splice(index, 1); setConfiguredTraits(newTraits); } @@ -219,9 +219,11 @@ type InputOptionArray = { const requireNoDuplicateTraits = (configuredTraits: TraitsOption[]) => (enteredTrait: Option) => () => { - let k = configuredTraits.map(trait => trait.traitKey.value.toLowerCase()); + const traitKey = configuredTraits.map(trait => + trait.traitKey.value.toLowerCase() + ); let occurance = 0; - k.forEach(key => { + traitKey.forEach(key => { if (key === enteredTrait.value.toLowerCase()) { occurance++; } diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 83cfe6bcb27db..bf9a3d68e6dd4 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -108,11 +108,11 @@ export type Props = { }; export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { - let newTrait = []; + const newTrait = []; for (let trait in allTraits) { - // if (!allTraits[trait][0]) { - // continue; - // } + if (!allTraits[trait][0]) { + continue; + } if (allTraits[trait].length > 0) { newTrait.push({ traitKey: { value: trait, label: trait }, From 7d4ecd8ed7807ce61c04b1ea64dafbce77eb2903 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 13:21:59 -0400 Subject: [PATCH 24/29] add maxWidth to DialogContent inner content to avoid flickering when scrollbar appears --- .../src/Users/UserAddEdit/UserAddEdit.tsx | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx index e8cf640919461..b2a1504a8fe7e 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/UserAddEdit.tsx @@ -17,7 +17,7 @@ */ import React from 'react'; -import { ButtonPrimary, ButtonSecondary, Alert } from 'design'; +import { ButtonPrimary, ButtonSecondary, Alert, Box } from 'design'; import Dialog, { DialogHeader, DialogTitle, @@ -88,40 +88,42 @@ export function UserAddEdit(props: ReturnType) { {attempt.status === 'failed' && ( )} - onChangeName(e.target.value)} - readonly={isNew ? false : true} - /> - onChangeRoles(values as Option[])} - noOptionsMessage={() => 'No roles found'} - loadOptions={async input => { - const roles = await fetchRoles(input); - return roles.map(r => ({ value: r, label: r })); - }} - elevated={true} - /> - + + onChangeName(e.target.value)} + readonly={isNew ? false : true} + /> + onChangeRoles(values as Option[])} + noOptionsMessage={() => 'No roles found'} + loadOptions={async input => { + const roles = await fetchRoles(input); + return roles.map(r => ({ value: r, label: r })); + }} + elevated={true} + /> + + Date: Thu, 13 Jun 2024 15:58:41 -0400 Subject: [PATCH 25/29] resolve review comments --- .../teleport/src/Users/UserAddEdit/TraitsEditor.tsx | 7 +++++++ web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx | 5 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index 3f9b3c8215e37..b1abce2de32be 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -58,6 +58,13 @@ export function TraitsEditor({ const newTraits = [...configuredTraits]; if (i.labelField === 'traitValues') { const traitValue: Option[] = i.option; + if (traitValue) { + traitValue.forEach((v, i) => { + if (v.value.trim() === '') { + traitValue.splice(i, 1); + } + }); + } newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitValue ?? [], diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index bf9a3d68e6dd4..0e99df4eecd60 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -34,7 +34,7 @@ export default function useUserDialog(props: Props) { label: r, })) ); - const [configuredTraits, setConfiguredTraits] = useState( + const [configuredTraits, setConfiguredTraits] = useState(() => traitsToTraitsOption(props.user.allTraits) ); @@ -47,7 +47,7 @@ export default function useUserDialog(props: Props) { } function onSave() { - let traitsToSave = {}; + const traitsToSave = {}; for (const traitKV of configuredTraits) { traitsToSave[traitKV.traitKey.value] = traitKV.traitValues.map( t => t.value @@ -89,7 +89,6 @@ export default function useUserDialog(props: Props) { fetchRoles: props.fetchRoles, setConfiguredTraits, isNew: props.isNew, - allTraits: props.user.allTraits, attempt, name, selectedRoles, From 47df76aabd5d3ddd5efd8969440bd26ca42f6c00 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 17:19:38 -0400 Subject: [PATCH 26/29] filter empty element only if length equals 1. add unit test --- .../Users/UserAddEdit/TraitsEditor.test.tsx | 18 ++++++++++ .../src/Users/UserAddEdit/useDialog.test.ts | 34 +++++++++++++++++++ .../src/Users/UserAddEdit/useDialog.tsx | 4 ++- 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx index e101eb354c871..0a89e6c79f344 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.test.tsx @@ -1,3 +1,21 @@ +/** + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + import React from 'react'; import { fireEvent, render, screen } from 'design/utils/testing'; diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts b/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts new file mode 100644 index 0000000000000..ebcb75522e1cb --- /dev/null +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts @@ -0,0 +1,34 @@ +/** + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { traitsToTraitsOption } from './useDialog'; + +describe('Test traitsToTraitsOption', () => { + test.each` + name | trait | expected + ${'trait with values (valid)'} | ${{ t: ['a', 'b'] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: 'a', value: 'a' }, { label: 'b', value: 'b' }] }]} + ${'empty trait (invalid)'} | ${{ t: [] }} | ${[]} + ${'trait with empty string (invalid)'} | ${{ t: [''] }} | ${[]} + ${'trait with null value (invalid)'} | ${{ t: [null] }} | ${[]} + ${'trait with first empty string (invalid)'} | ${{ t: ['', 'a'] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: '', value: '' }, { label: 'a', value: 'a' }] }]} + ${'trait with last empty element (valid)'} | ${{ t: ['a', ''] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: 'a', value: 'a' }, { label: '', value: '' }] }]} + `('$name', ({ trait, expected }) => { + const result = traitsToTraitsOption(trait); + expect(result).toEqual(expected); + }); +}); diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 0e99df4eecd60..4bb6b49a204de 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -110,7 +110,9 @@ export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { const newTrait = []; for (let trait in allTraits) { if (!allTraits[trait][0]) { - continue; + if (allTraits[trait].length === 1) { + continue; + } } if (allTraits[trait].length > 0) { newTrait.push({ From 1d38fb380b1465b91a7b0a724628a2df3f2e63ea Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 17:23:25 -0400 Subject: [PATCH 27/29] handlechange: return for empty string, trim whitespace --- .../src/Users/UserAddEdit/TraitsEditor.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx index b1abce2de32be..b4a8bab0c8966 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/TraitsEditor.tsx @@ -59,11 +59,13 @@ export function TraitsEditor({ if (i.labelField === 'traitValues') { const traitValue: Option[] = i.option; if (traitValue) { - traitValue.forEach((v, i) => { - if (v.value.trim() === '') { - traitValue.splice(i, 1); - } - }); + if (traitValue[traitValue.length - 1].value.trim() === '') { + return; + } + traitValue[traitValue.length - 1].label = + traitValue[traitValue.length - 1].label.trim(); + traitValue[traitValue.length - 1].value = + traitValue[traitValue.length - 1].value.trim(); } newTraits[i.index] = { ...newTraits[i.index], @@ -72,6 +74,8 @@ export function TraitsEditor({ setConfiguredTraits(newTraits); } else { const traitName: Option = i.option; + traitName.label = traitName.label.trim(); + traitName.value = traitName.value.trim(); newTraits[i.index] = { ...newTraits[i.index], [i.labelField]: traitName, From 1e80fcff976f4a4647a2eb6f1e1c8eabfd843d43 Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 17:34:25 -0400 Subject: [PATCH 28/29] explicitely handle null value --- web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts | 3 ++- web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts b/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts index ebcb75522e1cb..98669764ad021 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.test.ts @@ -24,7 +24,8 @@ describe('Test traitsToTraitsOption', () => { ${'trait with values (valid)'} | ${{ t: ['a', 'b'] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: 'a', value: 'a' }, { label: 'b', value: 'b' }] }]} ${'empty trait (invalid)'} | ${{ t: [] }} | ${[]} ${'trait with empty string (invalid)'} | ${{ t: [''] }} | ${[]} - ${'trait with null value (invalid)'} | ${{ t: [null] }} | ${[]} + ${'trait with null value (invalid)'} | ${{ t: null }} | ${[]} + ${'trait with null array (invalid)'} | ${{ t: [null] }} | ${[]} ${'trait with first empty string (invalid)'} | ${{ t: ['', 'a'] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: '', value: '' }, { label: 'a', value: 'a' }] }]} ${'trait with last empty element (valid)'} | ${{ t: ['a', ''] }} | ${[{ traitKey: { label: 't', value: 't' }, traitValues: [{ label: 'a', value: 'a' }, { label: '', value: '' }] }]} `('$name', ({ trait, expected }) => { diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 4bb6b49a204de..511c71b5ed375 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -109,6 +109,9 @@ export type Props = { export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { const newTrait = []; for (let trait in allTraits) { + if (!allTraits[trait]) { + continue; + } if (!allTraits[trait][0]) { if (allTraits[trait].length === 1) { continue; From 9a239baeec9b56cb7ef11787d92834dc0b107a4a Mon Sep 17 00:00:00 2001 From: flyinghermit Date: Thu, 13 Jun 2024 21:48:23 -0400 Subject: [PATCH 29/29] shorten empty string value check block --- web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx index 511c71b5ed375..acfe3451b6fe0 100644 --- a/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx +++ b/web/packages/teleport/src/Users/UserAddEdit/useDialog.tsx @@ -112,10 +112,8 @@ export function traitsToTraitsOption(allTraits: AllUserTraits): TraitsOption[] { if (!allTraits[trait]) { continue; } - if (!allTraits[trait][0]) { - if (allTraits[trait].length === 1) { - continue; - } + if (allTraits[trait].length === 1 && !allTraits[trait][0]) { + continue; } if (allTraits[trait].length > 0) { newTrait.push({