Web: Fix Discover setup access screen erroring when updating user

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

@@ -129,10 +129,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 },
+      },
+      'allTraits'
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as KubeMeta;
@@ -216,14 +219,17 @@ 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: {
-        ...result.current.dynamicTraits,
-        kubeGroups: ['dynamicKbGroup3', 'dynamicKbGroup4'],
-        kubeUsers: ['dynamicKbUser3', 'dynamicKbUser4'],
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: {
+          ...result.current.dynamicTraits,
+          kubeGroups: ['dynamicKbGroup3', 'dynamicKbGroup4'],
+          kubeUsers: ['dynamicKbUser3', 'dynamicKbUser4'],
+        },
       },
-    });
+      'allTraits'
+    );
   });
 
   test('database', async () => {
@@ -292,10 +298,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 },
+      },
+      'allTraits'
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as DbMeta;
@@ -368,14 +377,17 @@ 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: {
-        ...result.current.dynamicTraits,
-        databaseNames: ['banana', 'carrot'],
-        databaseUsers: ['apple'],
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: {
+          ...result.current.dynamicTraits,
+          databaseNames: ['banana', 'carrot'],
+          databaseUsers: ['apple'],
+        },
       },
-    });
+      'allTraits'
+    );
   });
 
   test('node', async () => {
@@ -436,10 +448,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 },
+      },
+      'allTraits'
+    );
 
     // Test that updating meta correctly updated the dynamic traits.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as NodeMeta;
@@ -521,13 +536,16 @@ 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,
-        awsRoleArns: [dynamicTraits.awsRoleArns[0]],
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: {
+          ...mockUser.traits,
+          awsRoleArns: [dynamicTraits.awsRoleArns[0]],
+        },
       },
-    });
+      'allTraits'
+    );
 
     // Test that app's awsRoles field got updated with the dynamic trait.
     const updatedMeta = spyUpdateAgentMeta.mock.results[0].value as AppMeta;
@@ -590,13 +608,16 @@ 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: {
-        ...result.current.dynamicTraits,
-        logins: ['banana', 'carrot'],
+    expect(teleCtx.userService.updateUser).toHaveBeenCalledWith(
+      {
+        ...mockUser,
+        traits: {
+          ...result.current.dynamicTraits,
+          logins: ['banana', 'carrot'],
+        },
       },
-    });
+      'allTraits'
+    );
   });
 });
 

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

@@ -328,13 +328,16 @@ export function useUserTraits() {
     setAttempt({ status: 'processing' });
     try {
       await ctx.userService
-        .updateUser({
-          ...user,
-          traits: {
-            ...user.traits,
-            ...newDynamicTraits,
+        .updateUser(
+          {
+            ...user,
+            traits: {
+              ...user.traits,
+              ...newDynamicTraits,
+            },
           },
-        })
+          'allTraits' /* exclude field */
+        )
         .catch((error: Error) => {
           emitErrorEvent(`error updating user traits: ${error.message}`);
           throw error;

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

@@ -77,15 +77,17 @@ 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, 'traits' /* exclude field */)
+      .then(result => {
+        setUsers([result, ...users.filter(i => i.name !== u.name)]);
+      });
   }
 
   async function onCreate(u: User) {
     const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true);
     return ctx.userService
-      .createUser(u, webauthnResponse)
+      .createUser(u, 'traits' /* exclude field */, webauthnResponse)
       .then(result => setUsers([result, ...users]))
       .then(() =>
         ctx.userService.createResetPasswordToken(

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

@@ -134,6 +134,10 @@ export interface User {
   allTraits?: AllUserTraits;
 }
 
+// Backend does not allow User field "traits" and "allTraits"
+// to be specified when creating or updating a user.
+export type ExludeUserField = 'traits' | '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

@@ -17,10 +17,11 @@
  */
 
 import api from 'teleport/services/api';
+import cfg from 'teleport/config';
 
 import user from './user';
 import { makeTraits } from './makeUser';
-import { PasswordState } from './types';
+import { PasswordState, User } from './types';
 
 test('undefined values in context response gives proper default values', async () => {
   const mockContext = {
@@ -370,3 +371,79 @@ 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, 'allTraits');
+  expect(api.put).toHaveBeenCalledWith(cfg.api.usersPath, {
+    name: 'name',
+    roles: [],
+    traits: blankTraits,
+  });
+
+  jest.clearAllMocks();
+
+  await user.updateUser(userReq, '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, 'allTraits');
+  expect(api.post).toHaveBeenCalledWith(
+    cfg.api.usersPath,
+    {
+      name: 'name',
+      roles: [],
+      traits: blankTraits,
+    },
+    null,
+    undefined
+  );
+
+  jest.clearAllMocks();
+
+  await user.createUser(userReq, 'traits');
+  expect(api.post).toHaveBeenCalledWith(
+    cfg.api.usersPath,
+    {
+      name: 'name',
+      roles: [],
+      allTraits: {},
+    },
+    null,
+    undefined
+  );
+});
+
+const blankTraits = {
+  logins: [],
+  databaseUsers: [],
+  databaseNames: [],
+  kubeUsers: [],
+  kubeGroups: [],
+  windowsLogins: [],
+  awsRoleArns: [],
+};

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

@@ -25,7 +25,7 @@ import { WebauthnAssertionResponse } from '../auth';
 import makeUserContext from './makeUserContext';
 import { makeResetToken } from './makeResetToken';
 import makeUser, { makeUsers } from './makeUser';
-import { User, UserContext, ResetPasswordType } from './types';
+import { User, UserContext, ResetPasswordType, ExludeUserField } from './types';
 
 const cache = {
   userContext: null as UserContext,
@@ -65,8 +65,10 @@ const service = {
    * @param user
    * @returns user
    */
-  updateUser(user: User) {
-    return api.put(cfg.getUsersUrl(), user).then(makeUser);
+  updateUser(user: User, excludeUserField: ExludeUserField) {
+    return api
+      .put(cfg.getUsersUrl(), withExcludedField(user, excludeUserField))
+      .then(makeUser);
   },
 
   /**
@@ -76,9 +78,18 @@ const service = {
    * @param user
    * @returns user
    */
-  createUser(user: User, webauthnResponse?: WebauthnAssertionResponse) {
+  createUser(
+    user: User,
+    excludeUserField: ExludeUserField,
+    webauthnResponse?: WebauthnAssertionResponse
+  ) {
     return api
-      .post(cfg.getUsersUrl(), user, null, webauthnResponse)
+      .post(
+        cfg.getUsersUrl(),
+        withExcludedField(user, excludeUserField),
+        null,
+        webauthnResponse
+      )
       .then(makeUser);
   },
 
@@ -118,4 +129,23 @@ const service = {
   },
 };
 
+function withExcludedField(user: User, excludeUserField: ExludeUserField) {
+  const userReq = { ...user };
+  switch (excludeUserField) {
+    case 'allTraits': {
+      delete userReq.allTraits;
+      break;
+    }
+    case 'traits': {
+      delete userReq.traits;
+      break;
+    }
+    default: {
+      excludeUserField satisfies never;
+    }
+  }
+
+  return userReq;
+}
+
 export default service;