Web: Fix Discover setup access screen erroring when updating user (#4…

…3558) (#43565)

* Web: fix discover user update error

* Address CRs

web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.test.tsx

@@ -29,6 +29,7 @@ import {
   defaultResourceSpec,
 } from 'teleport/Discover/Fixtures/fixtures';
 import TeleportContext from 'teleport/teleportContext';
+import { ExcludeUserField } from 'teleport/services/user';
 
 import { ResourceKind } from '../ResourceKind';
 
@@ -124,10 +125,13 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
 
     // Test that we are updating the user with the correct traits.
     const mockUser = getMockUser();
-    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith({
-      ...mockUser,
-      traits: { ...mockUser.traits, ...expected },
-    });
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: { ...mockUser.traits, ...expected },
+      },
+      ExcludeUserField.AllTraits
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as KubeMeta;
@@ -207,10 +211,13 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
 
     // Test that we are updating the user with the correct traits.
     const mockUser = getMockUser();
-    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith({
-      ...mockUser,
-      traits: { ...mockUser.traits, ...expected },
-    });
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: { ...mockUser.traits, ...expected },
+      },
+      ExcludeUserField.AllTraits
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as DbMeta;
@@ -283,15 +290,17 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
 
     // Test that we are updating the user with the correct traits.
     const mockUser = getMockUser();
-    const { databaseUsers, databaseNames } = result.current.dynamicTraits;
-    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith({
-      ...mockUser,
-      traits: {
-        ...result.current.dynamicTraits,
-        databaseNames: [...databaseNames, 'banana', 'carrot'],
-        databaseUsers: [...databaseUsers, 'apple'],
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: {
+          ...result.current.dynamicTraits,
+          databaseNames: ['banana', 'carrot'],
+          databaseUsers: ['apple'],
+        },
       },
-    });
+      ExcludeUserField.AllTraits
+    );
   });
 
   test('node', async () => {
@@ -352,10 +361,13 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
 
     // Test that we are updating the user with the correct traits.
     const mockUser = getMockUser();
-    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith({
-      ...mockUser,
-      traits: { ...mockUser.traits, ...expected },
-    });
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: { ...mockUser.traits, ...expected },
+      },
+      ExcludeUserField.AllTraits
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as NodeMeta;

web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.ts

@@ -21,11 +21,15 @@ import { arrayStrDiff } from 'teleport/lib/util';
 import useTeleport from 'teleport/useTeleport';
 import { Option } from 'teleport/Discover/Shared/SelectCreatable';
 import { useDiscover } from 'teleport/Discover/useDiscover';
+import {
+  ExcludeUserField,
+  type User,
+  type UserTraits,
+} from 'teleport/services/user';
 
 import { ResourceKind } from '../ResourceKind';
 
 import type { DbMeta, KubeMeta, NodeMeta } from 'teleport/Discover/useDiscover';
-import type { User, UserTraits } from 'teleport/services/user';
 
 // useUserTraits handles:
 //  - retrieving the latest user (for the dynamic traits) from the backend
@@ -152,19 +156,13 @@ export function useUserTraits() {
 
       case ResourceKind.Database:
         let newDynamicDbUsers = new Set<string>();
-        if (wantAutoDiscover) {
-          newDynamicDbUsers = new Set(dynamicTraits.databaseUsers);
-        }
         traitOpts.databaseUsers.forEach(o => {
           if (!staticTraits.databaseUsers.includes(o.value)) {
             newDynamicDbUsers.add(o.value);
           }
         });
 
         let newDynamicDbNames = new Set<string>();
-        if (wantAutoDiscover) {
-          newDynamicDbNames = new Set(dynamicTraits.databaseNames);
-        }
         traitOpts.databaseNames.forEach(o => {
           if (!staticTraits.databaseNames.includes(o.value)) {
             newDynamicDbNames.add(o.value);
@@ -259,13 +257,16 @@ export function useUserTraits() {
     setAttempt({ status: 'processing' });
     try {
       await ctx.userService
-        .updateUser({
-          ...user,
-          traits: {
-            ...user.traits,
-            ...newDynamicTraits,
+        .updateUser(
+          {
+            ...user,
+            traits: {
+              ...user.traits,
+              ...newDynamicTraits,
+            },
           },
-        })
+          ExcludeUserField.AllTraits
+        )
         .catch((error: Error) => {
           emitErrorEvent(`error updating user traits: ${error.message}`);
           throw error;

web/packages/teleport/src/Users/useUsers.ts

@@ -17,7 +17,7 @@
 import { ReactElement, useState, useEffect } from 'react';
 import { useAttempt } from 'shared/hooks';
 
-import { User } from 'teleport/services/user';
+import { ExcludeUserField, User } from 'teleport/services/user';
 import useTeleport from 'teleport/useTeleport';
 
 export default function useUsers({
@@ -74,14 +74,16 @@ export default function useUsers({
   }
 
   function onUpdate(u: User) {
-    return ctx.userService.updateUser(u).then(result => {
-      setUsers([result, ...users.filter(i => i.name !== u.name)]);
-    });
+    return ctx.userService
+      .updateUser(u, ExcludeUserField.Traits)
+      .then(result => {
+        setUsers([result, ...users.filter(i => i.name !== u.name)]);
+      });
   }
 
   function onCreate(u: User) {
     return ctx.userService
-      .createUser(u)
+      .createUser(u, ExcludeUserField.Traits)
       .then(result => setUsers([result, ...users]))
       .then(() => ctx.userService.createResetPasswordToken(u.name, 'invite'));
   }

web/packages/teleport/src/services/user/types.ts

@@ -112,6 +112,13 @@ export interface User {
   allTraits?: AllUserTraits;
 }
 
+// Backend does not allow User fields "traits" and "allTraits"
+// both to be specified in the same request when creating or updating a user.
+export enum ExcludeUserField {
+  Traits = 'traits',
+  AllTraits = 'allTraits',
+}
+
 // UserTraits contain fields that define traits for local accounts.
 export interface UserTraits {
   // logins is the list of logins that this user is allowed to

web/packages/teleport/src/services/user/user.test.ts

@@ -15,9 +15,11 @@
  */
 
 import api from 'teleport/services/api';
+import cfg from 'teleport/config';
 
 import user from './user';
 import { makeTraits } from './makeUser';
+import { ExcludeUserField, User } from './types';
 
 test('undefined values in context response gives proper default values', async () => {
   const mockContext = {
@@ -358,3 +360,69 @@ test('makeTraits', async () => {
     color: [],
   });
 });
+
+test('excludeUserFields when updating user', async () => {
+  // we are not testing the reply, so reply doesn't matter.
+  jest.spyOn(api, 'put').mockResolvedValue({} as any);
+
+  const userReq: User = {
+    name: 'name',
+    roles: [],
+    traits: blankTraits,
+    allTraits: {},
+  };
+
+  await user.updateUser(userReq, ExcludeUserField.AllTraits);
+  expect(api.put).toHaveBeenCalledWith(cfg.api.usersPath, {
+    name: 'name',
+    roles: [],
+    traits: blankTraits,
+  });
+
+  jest.clearAllMocks();
+
+  await user.updateUser(userReq, ExcludeUserField.Traits);
+  expect(api.put).toHaveBeenCalledWith(cfg.api.usersPath, {
+    name: 'name',
+    roles: [],
+    allTraits: {},
+  });
+});
+
+test('excludeUserFields when creating user', async () => {
+  // we are not testing the reply, so reply doesn't matter.
+  jest.spyOn(api, 'post').mockResolvedValue({} as any);
+
+  const userReq: User = {
+    name: 'name',
+    roles: [],
+    traits: blankTraits,
+    allTraits: {},
+  };
+
+  await user.createUser(userReq, ExcludeUserField.AllTraits);
+  expect(api.post).toHaveBeenCalledWith(cfg.api.usersPath, {
+    name: 'name',
+    roles: [],
+    traits: blankTraits,
+  });
+
+  jest.clearAllMocks();
+
+  await user.createUser(userReq, ExcludeUserField.Traits);
+  expect(api.post).toHaveBeenCalledWith(cfg.api.usersPath, {
+    name: 'name',
+    roles: [],
+    allTraits: {},
+  });
+});
+
+const blankTraits = {
+  logins: [],
+  databaseUsers: [],
+  databaseNames: [],
+  kubeUsers: [],
+  kubeGroups: [],
+  windowsLogins: [],
+  awsRoleArns: [],
+};

web/packages/teleport/src/services/user/user.ts

@@ -21,7 +21,12 @@ import session from 'teleport/services/websession';
 import makeUserContext from './makeUserContext';
 import { makeResetToken } from './makeResetToken';
 import makeUser, { makeUsers } from './makeUser';
-import { User, UserContext, ResetPasswordType } from './types';
+import {
+  User,
+  UserContext,
+  ResetPasswordType,
+  ExcludeUserField,
+} from './types';
 
 const cache = {
   userContext: null as UserContext,
@@ -61,8 +66,10 @@ const service = {
    * @param user
    * @returns user
    */
-  updateUser(user: User) {
-    return api.put(cfg.getUsersUrl(), user).then(makeUser);
+  updateUser(user: User, excludeUserField: ExcludeUserField) {
+    return api
+      .put(cfg.getUsersUrl(), withExcludedField(user, excludeUserField))
+      .then(makeUser);
   },
 
   /**
@@ -72,8 +79,10 @@ const service = {
    * @param user
    * @returns user
    */
-  createUser(user: User) {
-    return api.post(cfg.getUsersUrl(), user).then(makeUser);
+  createUser(user: User, excludeUserField: ExcludeUserField) {
+    return api
+      .post(cfg.getUsersUrl(), withExcludedField(user, excludeUserField))
+      .then(makeUser);
   },
 
   createResetPasswordToken(name: string, type: ResetPasswordType) {
@@ -103,4 +112,23 @@ const service = {
   },
 };
 
+function withExcludedField(user: User, excludeUserField: ExcludeUserField) {
+  const userReq = { ...user };
+  switch (excludeUserField) {
+    case ExcludeUserField.AllTraits: {
+      delete userReq.allTraits;
+      break;
+    }
+    case ExcludeUserField.Traits: {
+      delete userReq.traits;
+      break;
+    }
+    default: {
+      excludeUserField satisfies never;
+    }
+  }
+
+  return userReq;
+}
+
 export default service;