diff --git a/lib/auth/machineid/machineidv1/bot_service.go b/lib/auth/machineid/machineidv1/bot_service.go index 41794c444902d..5d319bf881067 100644 --- a/lib/auth/machineid/machineidv1/bot_service.go +++ b/lib/auth/machineid/machineidv1/bot_service.go @@ -527,6 +527,10 @@ func (bs *BotService) UpdateBot( opts := role.GetOptions() opts.MaxSessionTTL = types.Duration(req.Bot.Spec.MaxSessionTtl.AsDuration()) role.SetOptions(opts) + case "metadata.description": + meta := user.GetMetadata() + meta.Description = req.Bot.Metadata.Description + user.SetMetadata(meta) default: return nil, trace.BadParameter("update_mask: unsupported path %q", path) } diff --git a/lib/auth/machineid/machineidv1/machineidv1_test.go b/lib/auth/machineid/machineidv1/machineidv1_test.go index a356d49cdc963..8b6b310ea022d 100644 --- a/lib/auth/machineid/machineidv1/machineidv1_test.go +++ b/lib/auth/machineid/machineidv1/machineidv1_test.go @@ -663,7 +663,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "pre-existing", + Name: "pre-existing", + Description: "before", }, Spec: &machineidv1pb.BotSpec{ Roles: []string{beforeRole.GetName()}, @@ -708,7 +709,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: preExistingBot.Metadata.Name, + Name: preExistingBot.Metadata.Name, + Description: "after", }, Spec: &machineidv1pb.BotSpec{ Roles: []string{afterRole.GetName()}, @@ -728,7 +730,7 @@ func TestUpdateBot(t *testing.T) { }, }, UpdateMask: &fieldmaskpb.FieldMask{ - Paths: []string{"spec.roles", "spec.traits", "spec.max_session_ttl"}, + Paths: []string{"spec.roles", "spec.traits", "spec.max_session_ttl", "metadata.description"}, }, }, @@ -737,7 +739,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: preExistingBot.Metadata.Name, + Name: preExistingBot.Metadata.Name, + Description: "after", }, Spec: &machineidv1pb.BotSpec{ Roles: []string{afterRole.GetName()}, @@ -764,8 +767,9 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindUser, Version: types.V2, Metadata: types.Metadata{ - Name: preExistingBot.Status.UserName, - Namespace: defaults.Namespace, + Name: preExistingBot.Status.UserName, + Description: "after", + Namespace: defaults.Namespace, Labels: map[string]string{ types.BotLabel: preExistingBot.Metadata.Name, types.BotGenerationLabel: "1337", @@ -821,7 +825,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "valid-bot", + Name: "valid-bot", + Description: preExistingBot.Metadata.Description, }, Spec: &machineidv1pb.BotSpec{ Roles: []string{beforeRole.GetName()}, @@ -854,7 +859,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "bernard-lowe", + Name: "bernard-lowe", + Description: "before", }, Spec: nil, }, @@ -895,7 +901,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "", + Name: "", + Description: preExistingBot.Metadata.Description, }, Spec: &machineidv1pb.BotSpec{ Roles: []string{beforeRole.GetName()}, @@ -918,7 +925,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "foo", + Name: "foo", + Description: "before", }, Spec: &machineidv1pb.BotSpec{ Roles: []string{beforeRole.GetName()}, @@ -939,7 +947,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: "foo", + Name: "foo", + Description: preExistingBot.Metadata.Description, }, Spec: &machineidv1pb.BotSpec{ Roles: []string{beforeRole.GetName()}, @@ -962,7 +971,8 @@ func TestUpdateBot(t *testing.T) { Kind: types.KindBot, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: preExistingBot.Metadata.Name, + Name: preExistingBot.Metadata.Name, + Description: preExistingBot.Metadata.Description, }, Spec: &machineidv1pb.BotSpec{ Roles: []string{"foo", "", "bar"}, diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 5080bcd6b0e8d..433389618f3b5 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -1156,9 +1156,13 @@ func (h *Handler) bindDefaultEndpoints() { // PUT Machine ID bot by name // TODO(nicholasmarais1158) DELETE IN v20.0.0 // Replaced by `PUT /v2/webapi/sites/:site/machine-id/bot/:name` which allows editing more than just roles. - h.PUT("/webapi/sites/:site/machine-id/bot/:name", h.WithClusterAuth(h.updateBot)) + h.PUT("/webapi/sites/:site/machine-id/bot/:name", h.WithClusterAuth(h.updateBotV1)) // PUT Machine ID bot by name + // TODO(nicholasmarais1158) DELETE IN v20.0.0 + // Replaced by `PUT /v3/webapi/sites/:site/machine-id/bot/:name` which allows editing description. h.PUT("/v2/webapi/sites/:site/machine-id/bot/:name", h.WithClusterAuth(h.updateBotV2)) + // PUT Machine ID bot by name + h.PUT("/v3/webapi/sites/:site/machine-id/bot/:name", h.WithClusterAuth(h.updateBotV3)) // Delete Machine ID bot h.DELETE("/webapi/sites/:site/machine-id/bot/:name", h.WithClusterAuth(h.deleteBot)) diff --git a/lib/web/machineid.go b/lib/web/machineid.go index 9d644f4dd83c5..8d5a3f7de321d 100644 --- a/lib/web/machineid.go +++ b/lib/web/machineid.go @@ -17,6 +17,7 @@ package web import ( + "context" "net/http" "strconv" "strings" @@ -223,73 +224,86 @@ func (h *Handler) getBot(w http.ResponseWriter, r *http.Request, p httprouter.Pa // updateBot updates a bot with provided roles. The only supported change via this endpoint today is roles. // TODO(nicholasmarais1158) DELETE IN v20.0.0 - replaced by updateBotV2 // MUST delete with related code found in `web/packages/teleport/src/services/bot/bot.ts` -func (h *Handler) updateBot(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { - var request updateBotRequest +func (h *Handler) updateBotV1(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { + var request updateBotRequestV1 if err := httplib.ReadResourceJSON(r, &request); err != nil { return nil, trace.Wrap(err) } - botName := p.ByName("name") - if botName == "" { - return nil, trace.BadParameter("empty name") - } + return updateBot(r.Context(), p.ByName("name"), updateBotRequestV3{ + Roles: request.Roles, + }, sctx, cluster) +} - clt, err := sctx.GetUserClient(r.Context(), cluster) - if err != nil { - return nil, trace.Wrap(err) - } +type updateBotRequestV1 struct { + Roles []string `json:"roles"` +} - mask, err := fieldmaskpb.New(&machineidv1.Bot{}, "spec.roles") - if err != nil { +// updateBotV2 updates a bot with provided roles, traits and max_session_ttl. +// TODO(nicholasmarais1158) DELETE IN v20.0.0 - replaced by updateBotV3 +// MUST delete with related code found in `web/packages/teleport/src/services/bot/bot.ts` +func (h *Handler) updateBotV2(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { + var request updateBotRequestV2 + if err := httplib.ReadResourceJSON(r, &request); err != nil { return nil, trace.Wrap(err) } - updated, err := clt.BotServiceClient().UpdateBot(r.Context(), &machineidv1.UpdateBotRequest{ - UpdateMask: mask, - Bot: &machineidv1.Bot{ - Kind: types.KindBot, - Version: types.V1, - Metadata: &headerv1.Metadata{ - Name: botName, - }, - Spec: &machineidv1.BotSpec{ - Roles: request.Roles, - }, - }, - }) - if err != nil { - return nil, trace.Wrap(err, "unable to find existing bot") - } + return updateBot(r.Context(), p.ByName("name"), updateBotRequestV3{ + Roles: request.Roles, + Traits: request.Traits, + MaxSessionTtl: request.MaxSessionTtl, + }, sctx, cluster) +} - return updated, nil +type updateBotRequestV2 struct { + Roles []string `json:"roles"` + Traits []updateBotRequestTrait `json:"traits"` + MaxSessionTtl string `json:"max_session_ttl"` } -type updateBotRequest struct { - Roles []string `json:"roles"` +type updateBotRequestTrait struct { + Name string `json:"name"` + Values []string `json:"values"` } -// updateBotV2 updates a bot with provided roles, traits and max_session_ttl. -func (h *Handler) updateBotV2(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { - var request updateBotRequestV2 +// updateBot updates a bot with provided roles, traits, max_session_ttl and +// description. +func (h *Handler) updateBotV3(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { + var request updateBotRequestV3 if err := httplib.ReadResourceJSON(r, &request); err != nil { return nil, trace.Wrap(err) } - botName := p.ByName("name") - if botName == "" { - return nil, trace.BadParameter("empty name") - } + return updateBot(r.Context(), p.ByName("name"), request, sctx, cluster) +} - clt, err := sctx.GetUserClient(r.Context(), cluster) +type updateBotRequestV3 struct { + Roles []string `json:"roles"` + Traits []updateBotRequestTrait `json:"traits"` + MaxSessionTtl string `json:"max_session_ttl"` + Description *string `json:"description"` +} + +// updateBot updates a bot with provided roles, traits, max_session_ttl and +// description. +func updateBot(ctx context.Context, botName string, request updateBotRequestV3, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { + clt, err := sctx.GetUserClient(ctx, cluster) if err != nil { return nil, trace.Wrap(err) } + if botName == "" { + return nil, trace.BadParameter("empty name") + } + mask, err := fieldmaskpb.New(&machineidv1.Bot{}) if err != nil { return nil, trace.Wrap(err) } + metadata := headerv1.Metadata{ + Name: botName, + } spec := machineidv1.BotSpec{} if request.Roles != nil { @@ -323,15 +337,19 @@ func (h *Handler) updateBotV2(w http.ResponseWriter, r *http.Request, p httprout spec.MaxSessionTtl = durationpb.New(ttl) } - updated, err := clt.BotServiceClient().UpdateBot(r.Context(), &machineidv1.UpdateBotRequest{ + if request.Description != nil { + mask.Append(&machineidv1.Bot{}, "metadata.description") + + metadata.Description = *request.Description + } + + updated, err := clt.BotServiceClient().UpdateBot(ctx, &machineidv1.UpdateBotRequest{ UpdateMask: mask, Bot: &machineidv1.Bot{ - Kind: types.KindBot, - Version: types.V1, - Metadata: &headerv1.Metadata{ - Name: botName, - }, - Spec: &spec, + Kind: types.KindBot, + Version: types.V1, + Metadata: &metadata, + Spec: &spec, }, }) if err != nil { @@ -341,17 +359,6 @@ func (h *Handler) updateBotV2(w http.ResponseWriter, r *http.Request, p httprout return updated, nil } -type updateBotRequestV2 struct { - Roles []string `json:"roles"` - Traits []updateBotRequestTrait `json:"traits"` - MaxSessionTtl string `json:"max_session_ttl"` -} - -type updateBotRequestTrait struct { - Name string `json:"name"` - Values []string `json:"values"` -} - // getBotInstance retrieves a bot instance by id func (h *Handler) getBotInstance(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { botName := p.ByName("name") diff --git a/lib/web/machineid_test.go b/lib/web/machineid_test.go index 1f449f1fc2976..49aff0a007291 100644 --- a/lib/web/machineid_test.go +++ b/lib/web/machineid_test.go @@ -345,7 +345,7 @@ func TestEditBot(t *testing.T) { }) require.NoError(t, err) - response, err := pack.clt.PutJSON(ctx, fmt.Sprintf("%s/%s", endpoint, botName), updateBotRequest{ + response, err := pack.clt.PutJSON(ctx, fmt.Sprintf("%s/%s", endpoint, botName), updateBotRequestV1{ Roles: []string{"new-new-role"}, }) require.NoError(t, err) @@ -545,6 +545,71 @@ func TestEditBotMaxSessionTTL(t *testing.T) { assert.Equal(t, int64((1*time.Hour+2*time.Minute+3*time.Second)/time.Second), updatedBot.GetSpec().GetMaxSessionTtl().GetSeconds()) } +func TestEditBotDescription(t *testing.T) { + ctx := t.Context() + env := newWebPack(t, 1) + proxy := env.proxies[0] + pack := proxy.authPack(t, "admin", []types.Role{services.NewPresetEditorRole()}) + clusterName := env.server.ClusterName() + endpointV1 := pack.clt.Endpoint( + "v1", + "webapi", + "sites", + clusterName, + "machine-id", + "bot", + ) + endpointV3 := pack.clt.Endpoint( + "v3", + "webapi", + "sites", + clusterName, + "machine-id", + "bot", + ) + + // create a bot named `test-bot-edit` + botName := "test-bot-edit" + _, err := pack.clt.PostJSON(ctx, endpointV1, CreateBotRequest{ + BotName: botName, + Roles: []string{"test-role"}, + Traits: []*machineidv1.Trait{ + { + Name: "test-trait-1", + Values: []string{"value-1"}, + }, + }, + }) + require.NoError(t, err) + + response, err := pack.clt.Get(ctx, fmt.Sprintf("%s/%s", endpointV1, botName), nil) + require.NoError(t, err) + + var createdBot machineidv1.Bot + require.NoError(t, json.Unmarshal(response.Bytes(), &createdBot), "invalid response received") + assert.Equal(t, int64(43200), createdBot.GetSpec().GetMaxSessionTtl().GetSeconds()) + + description := "This is the bot's description." + response, err = pack.clt.PutJSON(ctx, fmt.Sprintf("%s/%s", endpointV3, botName), updateBotRequestV3{ + Description: &description, + }) + require.NoError(t, err) + + var updatedBot machineidv1.Bot + require.NoError(t, json.Unmarshal(response.Bytes(), &updatedBot), "invalid response received") + assert.Equal(t, http.StatusOK, response.Code(), "unexpected status code updating bot") + assert.Equal(t, botName, updatedBot.GetMetadata().GetName()) + assert.Equal(t, []string{"test-role"}, updatedBot.GetSpec().GetRoles()) + assert.Equal(t, []*machineidv1.Trait{ + { + Name: "test-trait-1", + Values: []string{"value-1"}, + }, + }, updatedBot.GetSpec().Traits) + assert.Equal(t, int64((12*time.Hour)/time.Second), updatedBot.GetSpec().GetMaxSessionTtl().GetSeconds()) + assert.Equal(t, description, updatedBot.GetMetadata().GetDescription()) +} + func TestListBotInstances(t *testing.T) { t.Parallel() diff --git a/web/packages/shared/components/Validation/rules.test.ts b/web/packages/shared/components/Validation/rules.test.ts index 827264949d559..40ffa0b628637 100644 --- a/web/packages/shared/components/Validation/rules.test.ts +++ b/web/packages/shared/components/Validation/rules.test.ts @@ -22,6 +22,7 @@ import { requiredEmailLike, requiredField, requiredIamRoleName, + requiredMaxLength, requiredPassword, requiredPort, requiredRoleArn, @@ -156,6 +157,25 @@ describe('requiredPort', () => { }); }); +describe('requiredMaxLength', () => { + const errMsg = 'message goes here'; + const validator = requiredMaxLength(errMsg, 10); + test.each` + value | expected + ${'Lorem ipsum'} | ${{ valid: false, message: errMsg }} + ${'Lorem ipsu'} | ${{ valid: true }} + ${' Lorem ipsu '} | ${{ valid: true }} + ${''} | ${{ valid: true }} + ${Array.from({ length: 11 })} | ${{ valid: false, message: errMsg }} + ${Array.from({ length: 10 })} | ${{ valid: true }} + ${[]} | ${{ valid: true }} + ${1} | ${{ valid: false, message: 'value must be a string or an array' }} + ${true} | ${{ valid: false, message: 'value must be a string or an array' }} + `('value: $value', ({ value, expected }) => { + expect(validator(value)()).toEqual(expected); + }); +}); + test('runRules', () => { expect( runRules( diff --git a/web/packages/shared/components/Validation/rules.ts b/web/packages/shared/components/Validation/rules.ts index cb6425369cd60..732a7fe0c6ec5 100644 --- a/web/packages/shared/components/Validation/rules.ts +++ b/web/packages/shared/components/Validation/rules.ts @@ -260,6 +260,39 @@ const requiredPort: Rule = port => () => { }; }; +/** + * requiredMaxLength checks for strings or arrays over a given length. + * + * @param message The custom error message to display to users. + * @param maxLength The maximum length to allow. + * @param value The value input. + */ +const requiredMaxLength = + ( + message: string, + maxLength: number + ): Rule => + value => + () => { + if (typeof value === 'string') { + const valid = value.trim().length <= maxLength; + return { + valid, + message: valid ? undefined : message, + }; + } + + if (Array.isArray(value)) { + const valid = value.length <= maxLength; + return { + valid, + message: valid ? undefined : message, + }; + } + + return { valid: false, message: 'value must be a string or an array' }; + }; + /** * A rule function that combines multiple inner rule functions. All rules must * return `valid`, otherwise it returns a comma separated string containing all @@ -370,6 +403,7 @@ export { requiredRoleArn, requiredIamRoleName, requiredEmailLike, + requiredMaxLength, requiredAll, requiredMatchingRoleNameAndRoleArn, validAwsIAMRoleName, diff --git a/web/packages/teleport/src/Bots/Details/BotDetails.story.tsx b/web/packages/teleport/src/Bots/Details/BotDetails.story.tsx index d3be0a9c70042..e5e0bb3f4f7e9 100644 --- a/web/packages/teleport/src/Bots/Details/BotDetails.story.tsx +++ b/web/packages/teleport/src/Bots/Details/BotDetails.story.tsx @@ -107,7 +107,9 @@ export const Happy: Story = { })), }), listV2LocksSuccess(), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), removeLockSuccess(), createLockSuccess(), deleteBotSuccess(), @@ -145,7 +147,9 @@ export const HappyWithEmpty: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -192,7 +196,9 @@ export const HappyWithTypical: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -205,6 +211,8 @@ export const HappyWithLongValues: Story = { handlers: [ getBotSuccess({ name: 'ansibleworkeransibleworkeransibleworkeransibleworkeransibleworkeransibleworker', + description: + 'This is a bot. This is a bot. This is a bot. This is a bot. This is a bot.', roles: [ 'rolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerolerole', ], @@ -255,7 +263,9 @@ export const HappyWithLongValues: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -281,7 +291,9 @@ export const HappyWithoutEditPermission: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -307,7 +319,9 @@ export const HappyWithoutTokenListPermission: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -331,7 +345,9 @@ export const HappyWithMFAPrompt: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -354,7 +370,9 @@ export const HappyWithTokensError: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -385,7 +403,9 @@ export const HappyWithTokensOutdatedProxy: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -411,7 +431,9 @@ export const HappyWithoutBotInstanceListPermission: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), listV2LocksSuccess(), ], }, @@ -448,7 +470,9 @@ export const HappyWithLock: Story = { }, ], }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), removeLockSuccess(), createLockSuccess(), ], @@ -476,7 +500,9 @@ export const HappyWithLockError: Story = { })), }), listV2LocksError(500, 'error message goes here'), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), ], }, }, diff --git a/web/packages/teleport/src/Bots/Details/BotDetails.test.tsx b/web/packages/teleport/src/Bots/Details/BotDetails.test.tsx index 8264fbd27a137..7910dcb65226f 100644 --- a/web/packages/teleport/src/Bots/Details/BotDetails.test.tsx +++ b/web/packages/teleport/src/Bots/Details/BotDetails.test.tsx @@ -46,6 +46,7 @@ import { defaultAccess, makeAcl } from 'teleport/services/user/makeAcl'; import { listBotInstancesSuccess } from 'teleport/test/helpers/botInstances'; import { deleteBotSuccess, + EditBotApiVersion, editBotSuccess, getBotError, getBotSuccess, @@ -147,6 +148,9 @@ describe('BotDetails', () => { expect(panel).toBeInTheDocument(); expect(within(panel!).getByText('test-bot-name')).toBeInTheDocument(); + expect( + within(panel!).getByText("This is the bot's description.") + ).toBeInTheDocument(); expect(within(panel!).getByText('12h')).toBeInTheDocument(); }); @@ -363,7 +367,7 @@ describe('BotDetails', () => { // Change something to enable the save button await inputMaxSessionDuration(user, '12h 30m'); - withSaveSuccess(2, { + withSaveSuccess('v2', { roles: ['role-1'], traits: [ { @@ -765,12 +769,12 @@ function withFetchInstancesSuccess() { ); } -const withSaveSuccess = ( - version: 1 | 2 = 2, +function withSaveSuccess( + version: EditBotApiVersion = 'v3', overrides?: Partial -) => { +) { server.use(editBotSuccess(version, overrides)); -}; +} function withFetchRolesSuccess() { server.use( diff --git a/web/packages/teleport/src/Bots/Details/BotDetails.tsx b/web/packages/teleport/src/Bots/Details/BotDetails.tsx index e20df3ce88ee5..c228ec0daf283 100644 --- a/web/packages/teleport/src/Bots/Details/BotDetails.tsx +++ b/web/packages/teleport/src/Bots/Details/BotDetails.tsx @@ -243,12 +243,16 @@ export function BotDetails() { {data.name} + Description + {data.description || '-'} Max session duration - {data.max_session_ttl - ? formatDuration(data.max_session_ttl, { - separator: ' ', - }) - : '-'} + + {data.max_session_ttl + ? formatDuration(data.max_session_ttl, { + separator: ' ', + }) + : '-'} + diff --git a/web/packages/teleport/src/Bots/Edit/EditDialog.story.tsx b/web/packages/teleport/src/Bots/Edit/EditDialog.story.tsx index 8a702b9d70ea1..6b47ef2f154c7 100644 --- a/web/packages/teleport/src/Bots/Edit/EditDialog.story.tsx +++ b/web/packages/teleport/src/Bots/Edit/EditDialog.story.tsx @@ -83,7 +83,9 @@ export const Happy: Story = { kind: 'role', })), }), - editBotSuccess(), + editBotSuccess('v1'), + editBotSuccess('v2'), + editBotSuccess('v3'), ], }, }, @@ -169,7 +171,9 @@ export const WithSubmitFailure: Story = { kind: 'role', })), }), - editBotError(500, 'something went wrong'), + editBotError('v1', 500, 'something went wrong'), + editBotError('v2', 500, 'something went wrong'), + editBotError('v3', 500, 'something went wrong'), ], }, }, @@ -189,7 +193,25 @@ export const WithSubmitOutdatedProxy: Story = { kind: 'role', })), }), - editBotError(404, 'path not found', { + editBotError('v1', 404, 'path not found', { + proxyVersion: { + major: 19, + minor: 0, + patch: 0, + preRelease: 'dev', + string: '18.0.0', + }, + }), + editBotError('v2', 404, 'path not found', { + proxyVersion: { + major: 19, + minor: 0, + patch: 0, + preRelease: 'dev', + string: '18.0.0', + }, + }), + editBotError('v3', 404, 'path not found', { proxyVersion: { major: 19, minor: 0, diff --git a/web/packages/teleport/src/Bots/Edit/EditDialog.test.tsx b/web/packages/teleport/src/Bots/Edit/EditDialog.test.tsx index 8caa9dd4da67c..d9129588a83ce 100644 --- a/web/packages/teleport/src/Bots/Edit/EditDialog.test.tsx +++ b/web/packages/teleport/src/Bots/Edit/EditDialog.test.tsx @@ -17,6 +17,7 @@ */ import { QueryClientProvider } from '@tanstack/react-query'; +import { UserEvent } from '@testing-library/user-event'; import { setupServer } from 'msw/node'; import { PropsWithChildren } from 'react'; import selectEvent from 'react-select-event'; @@ -25,11 +26,10 @@ import darkTheme from 'design/theme/themes/darkTheme'; import { ConfiguredThemeProvider } from 'design/ThemeProvider'; import { act, - fireEvent, render, screen, testQueryClient, - waitFor, + userEvent, waitForElementToBeRemoved, } from 'design/utils/testing'; @@ -38,6 +38,7 @@ import { createTeleportContext } from 'teleport/mocks/contexts'; import { EditBotRequest, FlatBot } from 'teleport/services/bot/types'; import { defaultAccess, makeAcl } from 'teleport/services/user/makeAcl'; import { + EditBotApiVersion, editBotError, editBotSuccess, getBotError, @@ -126,16 +127,15 @@ describe('EditDialog', () => { withFetchBotSuccess(); withFetchRolesSuccess({ items: ['test-role'] }); - renderComponent({ onSuccess }); + const { user } = renderComponent({ onSuccess }); await waitForLoading(); await inputRole('test-role'); - withSaveSuccess(1); + withSaveSuccess('v1'); const saveButton = screen.getByRole('button', { name: 'Save' }); expect(saveButton).toBeEnabled(); - fireEvent.click(saveButton); - await waitForSaveButton(); + await user.click(saveButton); expect(onSuccess).toHaveBeenCalledTimes(1); expect(onSuccess).toHaveBeenLastCalledWith({ @@ -167,21 +167,20 @@ describe('EditDialog', () => { withFetchBotSuccess(); withFetchRolesSuccess(); - renderComponent({ onSuccess }); + const { user } = renderComponent({ onSuccess }); await waitForLoading(); const addTraitButton = screen.getByRole('button', { name: 'Add another trait', }); - fireEvent.click(addTraitButton); + await user.click(addTraitButton); - await inputTrait('logins', ['test-value']); + await inputTrait(user, 'logins', ['test-value']); - withSaveSuccess(); + withSaveSuccess('v2'); const saveButton = screen.getByRole('button', { name: 'Save' }); expect(saveButton).toBeEnabled(); - fireEvent.click(saveButton); - await waitForSaveButton(); + await user.click(saveButton); expect(onSuccess).toHaveBeenCalledTimes(1); expect(onSuccess).toHaveBeenLastCalledWith({ @@ -217,16 +216,15 @@ describe('EditDialog', () => { withFetchBotSuccess(); withFetchRolesSuccess(); - renderComponent({ onSuccess }); + const { user } = renderComponent({ onSuccess }); await waitForLoading(); - await inputMaxSessionDuration(' 12h 30m '); + await inputMaxSessionDuration(user, ' 12h 30m '); - withSaveSuccess(); + withSaveSuccess('v2'); const saveButton = screen.getByRole('button', { name: 'Save' }); expect(saveButton).toBeEnabled(); - fireEvent.click(saveButton); - await waitForSaveButton(); + await user.click(saveButton); expect(onSuccess).toHaveBeenCalledTimes(1); expect(onSuccess).toHaveBeenLastCalledWith({ @@ -253,22 +251,63 @@ describe('EditDialog', () => { }); }); + it('should allow description to be edited', async () => { + const onSuccess = jest.fn(); + + withFetchBotSuccess({ + description: '', + }); + withFetchRolesSuccess(); + const { user } = renderComponent({ onSuccess }); + await waitForLoading(); + + await inputDescription(user, 'Hello world!'); + + withSaveSuccess(); + const saveButton = screen.getByRole('button', { name: 'Save' }); + expect(saveButton).toBeEnabled(); + await user.click(saveButton); + + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(onSuccess).toHaveBeenLastCalledWith({ + description: 'Hello world!', + kind: 'bot', + labels: new Map(), + max_session_ttl: { + seconds: 43200, + }, + name: 'test-bot-name', + namespace: '', + revision: '', + roles: ['admin', 'user'], + status: 'active', + subKind: '', + traits: [ + { + name: 'trait-1', + values: ['value-1', 'value-2', 'value-3'], + }, + ], + type: null, + version: 'v1', + }); + }); + it('should show a save error state', async () => { const onSuccess = jest.fn(); withFetchBotSuccess(); withFetchRolesSuccess(); - renderComponent({ onSuccess }); + const { user } = renderComponent({ onSuccess }); await waitForLoading(); // Change something to enable the save button - await inputMaxSessionDuration('12h 30m'); + await inputMaxSessionDuration(user, '12h 30m'); - withSaveError(); + withSaveError('v2'); const saveButton = screen.getByRole('button', { name: 'Save' }); expect(saveButton).toBeEnabled(); - fireEvent.click(saveButton); - await waitForSaveButton(); + await user.click(saveButton); expect(screen.getByText('something went wrong')).toBeInTheDocument(); @@ -280,18 +319,18 @@ describe('EditDialog', () => { withFetchBotSuccess(); withFetchRolesSuccess({ items: ['test-role'] }); - renderComponent({ onSuccess }); + const { user } = renderComponent({ onSuccess }); await waitForLoading(); await inputRole('test-role'); - await inputTrait('logins', ['test-value']); - await inputMaxSessionDuration('12h 30m'); + await inputTrait(user, 'logins', ['test-value']); + await inputMaxSessionDuration(user, '12h 30m'); + await inputDescription(user, 'hello'); withSaveVersionMismatch(); const saveButton = screen.getByRole('button', { name: 'Save' }); expect(saveButton).toBeEnabled(); - fireEvent.click(saveButton); - await waitForSaveButton(); + await user.click(saveButton); expect(onSuccess).not.toHaveBeenCalled(); @@ -307,7 +346,7 @@ async function inputRole(role: string) { await selectEvent.select(screen.getByLabelText('Roles'), [role]); } -async function inputTrait(name: string, values: string[]) { +async function inputTrait(user: UserEvent, name: string, values: string[]) { await selectEvent.select(screen.getAllByLabelText('trait-key').at(-1)!, [ name, ]); @@ -315,29 +354,41 @@ async function inputTrait(name: string, values: string[]) { const traitValue = screen.getAllByLabelText('trait-values'); for (const value of values) { - fireEvent.change(traitValue.at(-1)!, { - target: { value: value }, - }); - fireEvent.keyDown(traitValue.at(-1)!, { key: 'Enter' }); + const input = traitValue.at(-1); + await user.clear(input!); + await user.type(input!, value + '{enter}'); } } -async function inputMaxSessionDuration(duration: string) { - const ttlInput = screen.getByLabelText('Max session duration'); - fireEvent.change(ttlInput, { target: { value: duration } }); +async function inputMaxSessionDuration(user: UserEvent, duration: string) { + const input = screen.getByLabelText('Max session duration'); + await user.clear(input); + await user.click(input); + await user.paste(duration); +} + +async function inputDescription(user: UserEvent, description: string) { + const input = screen.getByLabelText('Description'); + await user.clear(input); + await user.click(input); + await user.paste(description); } function withFetchBotError(status = 500, message = 'something went wrong') { server.use(getBotError(status, message)); } -function withSaveError(status = 500, message = 'something went wrong') { - server.use(editBotError(status, message)); +function withSaveError( + version: EditBotApiVersion = 'v3', + status = 500, + message = 'something went wrong' +) { + server.use(editBotError(version, status, message)); } -function withSaveVersionMismatch() { +function withSaveVersionMismatch(version: EditBotApiVersion = 'v3') { server.use( - editBotError(404, 'path not found', { + editBotError(version, 404, 'path not found', { proxyVersion: { major: 19, minor: 0, @@ -349,12 +400,12 @@ function withSaveVersionMismatch() { ); } -function withFetchBotSuccess() { - server.use(getBotSuccess()); +function withFetchBotSuccess(...params: Parameters) { + server.use(getBotSuccess(...params)); } function withSaveSuccess( - version: 1 | 2 = 2, + version: EditBotApiVersion = 'v3', overrides?: Partial ) { server.use(editBotSuccess(version, overrides)); @@ -385,27 +436,24 @@ function renderComponent(options?: { onSuccess = jest.fn(), customAcl, } = options ?? {}; - return render( - , - { wrapper: makeWrapper({ customAcl }) } - ); + const user = userEvent.setup(); + return { + ...render( + , + { wrapper: makeWrapper({ customAcl }) } + ), + user, + }; } async function waitForLoading() { return waitForElementToBeRemoved(() => screen.queryByTestId('loading')); } -async function waitForSaveButton() { - return waitFor(() => { - const saveButton = screen.getByRole('button', { name: 'Save' }); - expect(saveButton).toBeEnabled(); - }); -} - function makeWrapper(params?: { customAcl?: ReturnType }) { const { customAcl = makeAcl({ diff --git a/web/packages/teleport/src/Bots/Edit/EditDialog.tsx b/web/packages/teleport/src/Bots/Edit/EditDialog.tsx index 29aa5ec958719..e684a41747273 100644 --- a/web/packages/teleport/src/Bots/Edit/EditDialog.tsx +++ b/web/packages/teleport/src/Bots/Edit/EditDialog.tsx @@ -35,13 +35,17 @@ import Dialog, { } from 'design/DialogConfirmation'; import FieldInput from 'shared/components/FieldInput'; import { FieldSelectAsync } from 'shared/components/FieldSelect'; +import { FieldTextArea } from 'shared/components/FieldTextArea/FieldTextArea'; import { Option } from 'shared/components/Select'; import { TraitsEditor, TraitsOption, } from 'shared/components/TraitsEditor/TraitsEditor'; import Validation from 'shared/components/Validation'; -import { requiredField } from 'shared/components/Validation/rules'; +import { + requiredField, + requiredMaxLength, +} from 'shared/components/Validation/rules'; import { editBot, fetchRoles } from 'teleport/services/bot/bot'; import { FlatBot } from 'teleport/services/bot/types'; @@ -69,6 +73,10 @@ export function EditDialog(props: { const [selectedMaxSessionDuration, setSelectedMaxSessionDuration] = useState< string | null >(null); + const [selectedDescription, setSelectedDescription] = useState( + null + ); + const { isSuccess, data, error, isLoading } = useGetBot( { botName }, { @@ -104,11 +112,13 @@ export function EditDialog(props: { })) ?? null; const max_session_ttl = selectedMaxSessionDuration?.trim().replaceAll(' ', '') ?? null; + const description = selectedDescription?.trim(); const req = { roles, traits, max_session_ttl, + description, }; mutate({ botName, req }); @@ -117,7 +127,8 @@ export function EditDialog(props: { const isDirty = selectedRoles !== null || selectedTraits !== null || - selectedMaxSessionDuration !== null; + selectedMaxSessionDuration !== null || + selectedDescription !== null; const missingPermissions = [ ...(hasReadPermission ? [] : ['bots.read']), @@ -189,6 +200,17 @@ export function EditDialog(props: { readonly={true} helperText={'Bot name cannot be changed'} /> + setSelectedDescription(e.target.value)} + rule={requiredMaxLength( + 'Description must be 200 characters or shorter.', + 200 + )} + helperText={'200 characters maximum'} + /> { + if (!['roles', 'traits', 'max_session_ttl'].includes(key)) { + return value === null || value === undefined; + } + return true; + }); +} diff --git a/web/packages/teleport/src/services/bot/types.ts b/web/packages/teleport/src/services/bot/types.ts index 255b9a1fc1a17..f42d7612b281c 100644 --- a/web/packages/teleport/src/services/bot/types.ts +++ b/web/packages/teleport/src/services/bot/types.ts @@ -131,4 +131,6 @@ export type EditBotRequest = { traits?: ApiBotTrait[] | null; // max_session_ttl is the maximum session TTL max_session_ttl?: string | null; + // description is the bot's metadata description + description?: string | null; }; diff --git a/web/packages/teleport/src/test/helpers/bots.ts b/web/packages/teleport/src/test/helpers/bots.ts index 9fdf4dce02594..4d78f237c5b8b 100644 --- a/web/packages/teleport/src/test/helpers/bots.ts +++ b/web/packages/teleport/src/test/helpers/bots.ts @@ -24,12 +24,14 @@ import { JsonObject } from 'teleport/types'; export const getBotSuccess = (overrides?: { name?: ApiBot['metadata']['name']; + description?: ApiBot['metadata']['description']; roles?: ApiBot['spec']['roles']; traits?: ApiBot['spec']['traits']; max_session_ttl?: ApiBot['spec']['max_session_ttl']; }) => { const { name = 'test-bot-name', + description = "This is the bot's description.", roles = ['admin', 'user'], traits = [ { @@ -50,7 +52,7 @@ export const getBotSuccess = (overrides?: { version: 'v1', metadata: { name, - description: '', + description, labels: new Map(), namespace: '', revision: '', @@ -72,17 +74,22 @@ export const getBotSuccess = (overrides?: { * @returns http handler to use in SetupServerApi.use() */ export const editBotSuccess = ( - version: 1 | 2 = 2, + version: EditBotApiVersion = 'v3', overrides?: Partial ) => http.put<{ botName: string }>( - version === 1 ? cfg.api.bot.update : cfg.api.bot.updateV2, + version === 'v1' + ? cfg.api.bot.update + : version === 'v2' + ? cfg.api.bot.updateV2 + : cfg.api.bot.updateV3, async ({ request, params }) => { const req = (await request.clone().json()) as EditBotRequest; const { roles = req.roles, traits = req.traits, max_session_ttl = req.max_session_ttl, + description = req.description, } = overrides ?? {}; const maxSessionTtlSeconds = @@ -95,7 +102,7 @@ export const editBotSuccess = ( version: 'v1', metadata: { name: params.botName, - description: '', + description, labels: new Map(), namespace: '', revision: '', @@ -132,13 +139,24 @@ export const getBotError = (status: number, error: string | null = null) => }); export const editBotError = ( + version: EditBotApiVersion = 'v3', status: number, error: string | null = null, fields: JsonObject = {} ) => - http.put(cfg.api.bot.updateV2, () => { - return HttpResponse.json({ error: { message: error }, fields }, { status }); - }); + http.put( + version === 'v1' + ? cfg.api.bot.update + : version === 'v2' + ? cfg.api.bot.updateV2 + : cfg.api.bot.updateV3, + () => { + return HttpResponse.json( + { error: { message: error }, fields }, + { status } + ); + } + ); export const getBotForever = () => http.get( @@ -157,3 +175,5 @@ export const editBotForever = () => /* never resolved */ }) ); + +export type EditBotApiVersion = 'v1' | 'v2' | 'v3';