Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Address CRs
Browse files Browse the repository at this point in the history
kimlisa authored and github-actions committed Jun 26, 2024
1 parent 35e56be commit e194dc7
Showing 6 changed files with 39 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ import {
} from 'teleport/Discover/Fixtures/fixtures';
import TeleportContext from 'teleport/teleportContext';
import { app } from 'teleport/Discover/AwsMangementConsole/fixtures';
import { ExcludeUserField } from 'teleport/services/user';

import { ResourceKind } from '../ResourceKind';

@@ -134,7 +135,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
...mockUser,
traits: { ...mockUser.traits, ...expected },
},
'allTraits'
ExcludeUserField.AllTraits
);

// Test that updating meta correctly updated the dynamic traits.
@@ -228,7 +229,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
kubeUsers: ['dynamicKbUser3', 'dynamicKbUser4'],
},
},
'allTraits'
ExcludeUserField.AllTraits
);
});

@@ -303,7 +304,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
...mockUser,
traits: { ...mockUser.traits, ...expected },
},
'allTraits'
ExcludeUserField.AllTraits
);

// Test that updating meta correctly updated the dynamic traits.
@@ -386,7 +387,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
databaseUsers: ['apple'],
},
},
'allTraits'
ExcludeUserField.AllTraits
);
});

@@ -453,7 +454,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
...mockUser,
traits: { ...mockUser.traits, ...expected },
},
'allTraits'
ExcludeUserField.AllTraits
);

// Test that updating meta correctly updated the dynamic traits.
@@ -544,7 +545,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
awsRoleArns: [dynamicTraits.awsRoleArns[0]],
},
},
'allTraits'
ExcludeUserField.AllTraits
);

// Test that app's awsRoles field got updated with the dynamic trait.
@@ -616,7 +617,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta,
logins: ['banana', 'carrot'],
},
},
'allTraits'
ExcludeUserField.AllTraits
);
});
});
Original file line number Diff line number Diff line change
@@ -24,6 +24,11 @@ import useTeleport from 'teleport/useTeleport';
import { Option } from 'teleport/Discover/Shared/SelectCreatable';
import { useDiscover } from 'teleport/Discover/useDiscover';
import { splitAwsIamArn } from 'teleport/services/integrations/aws';
import {
ExcludeUserField,
type User,
type UserTraits,
} from 'teleport/services/user';

import { ResourceKind } from '../ResourceKind';

@@ -33,7 +38,6 @@ import type {
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
@@ -336,7 +340,7 @@ export function useUserTraits() {
...newDynamicTraits,
},
},
'allTraits' /* exclude field */
ExcludeUserField.AllTraits
)
.catch((error: Error) => {
emitErrorEvent(`error updating user traits: ${error.message}`);
6 changes: 3 additions & 3 deletions web/packages/teleport/src/Users/useUsers.ts
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@
import { ReactElement, useEffect, useState } 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';
import auth from 'teleport/services/auth/auth';

@@ -78,7 +78,7 @@ export default function useUsers({

function onUpdate(u: User) {
return ctx.userService
.updateUser(u, 'traits' /* exclude field */)
.updateUser(u, ExcludeUserField.Traits)
.then(result => {
setUsers([result, ...users.filter(i => i.name !== u.name)]);
});
@@ -87,7 +87,7 @@ export default function useUsers({
async function onCreate(u: User) {
const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true);
return ctx.userService
.createUser(u, 'traits' /* exclude field */, webauthnResponse)
.createUser(u, ExcludeUserField.Traits, webauthnResponse)
.then(result => setUsers([result, ...users]))
.then(() =>
ctx.userService.createResetPasswordToken(
9 changes: 6 additions & 3 deletions web/packages/teleport/src/services/user/types.ts
Original file line number Diff line number Diff line change
@@ -135,9 +135,12 @@ 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';
// 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 {
10 changes: 5 additions & 5 deletions web/packages/teleport/src/services/user/user.test.ts
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ import cfg from 'teleport/config';

import user from './user';
import { makeTraits } from './makeUser';
import { PasswordState, User } from './types';
import { ExcludeUserField, PasswordState, User } from './types';

test('undefined values in context response gives proper default values', async () => {
const mockContext = {
@@ -390,7 +390,7 @@ test('excludeUserFields when updating user', async () => {
allTraits: {},
};

await user.updateUser(userReq, 'allTraits');
await user.updateUser(userReq, ExcludeUserField.AllTraits);
expect(api.put).toHaveBeenCalledWith(cfg.api.usersPath, {
name: 'name',
roles: [],
@@ -399,7 +399,7 @@ test('excludeUserFields when updating user', async () => {

jest.clearAllMocks();

await user.updateUser(userReq, 'traits');
await user.updateUser(userReq, ExcludeUserField.Traits);
expect(api.put).toHaveBeenCalledWith(cfg.api.usersPath, {
name: 'name',
roles: [],
@@ -418,7 +418,7 @@ test('excludeUserFields when creating user', async () => {
allTraits: {},
};

await user.createUser(userReq, 'allTraits');
await user.createUser(userReq, ExcludeUserField.AllTraits);
expect(api.post).toHaveBeenCalledWith(
cfg.api.usersPath,
{
@@ -432,7 +432,7 @@ test('excludeUserFields when creating user', async () => {

jest.clearAllMocks();

await user.createUser(userReq, 'traits');
await user.createUser(userReq, ExcludeUserField.Traits);
expect(api.post).toHaveBeenCalledWith(
cfg.api.usersPath,
{
17 changes: 11 additions & 6 deletions web/packages/teleport/src/services/user/user.ts
Original file line number Diff line number Diff line change
@@ -25,7 +25,12 @@ import { WebauthnAssertionResponse } from '../auth';
import makeUserContext from './makeUserContext';
import { makeResetToken } from './makeResetToken';
import makeUser, { makeUsers } from './makeUser';
import { User, UserContext, ResetPasswordType, ExludeUserField } from './types';
import {
User,
UserContext,
ResetPasswordType,
ExcludeUserField,
} from './types';

const cache = {
userContext: null as UserContext,
@@ -65,7 +70,7 @@ const service = {
* @param user
* @returns user
*/
updateUser(user: User, excludeUserField: ExludeUserField) {
updateUser(user: User, excludeUserField: ExcludeUserField) {
return api
.put(cfg.getUsersUrl(), withExcludedField(user, excludeUserField))
.then(makeUser);
@@ -80,7 +85,7 @@ const service = {
*/
createUser(
user: User,
excludeUserField: ExludeUserField,
excludeUserField: ExcludeUserField,
webauthnResponse?: WebauthnAssertionResponse
) {
return api
@@ -129,14 +134,14 @@ const service = {
},
};

function withExcludedField(user: User, excludeUserField: ExludeUserField) {
function withExcludedField(user: User, excludeUserField: ExcludeUserField) {
const userReq = { ...user };
switch (excludeUserField) {
case 'allTraits': {
case ExcludeUserField.AllTraits: {
delete userReq.allTraits;
break;
}
case 'traits': {
case ExcludeUserField.Traits: {
delete userReq.traits;
break;
}

0 comments on commit e194dc7

Please sign in to comment.