From c8d24fdd6137c2302b3130dc982900b63d0d6639 Mon Sep 17 00:00:00 2001 From: Lion Chen Date: Mon, 8 Jun 2026 12:18:52 -0700 Subject: [PATCH] Surface User Display Names in the Web Users Page and User Details (#67406) * Surface user display names in web Users page and user details * Refactor user authentication type display and update related tests * reduce code churn * add 16px back * Refactor UserDisplayName component for improved layout and update related tests * Refactor UserDetails tests to improve rendering checks and remove unnecessary assertions --- .../UserDisplayName/UserDisplayName.story.tsx | 26 +++--- .../UserDisplayName/UserDisplayName.test.tsx | 61 ++++++++++-- .../UserDisplayName/UserDisplayName.tsx | 92 +++++++++++++------ .../Users/UserDetails/UserDetails.story.tsx | 12 +++ .../Users/UserDetails/UserDetails.test.tsx | 91 ++++++++++++++++++ .../src/Users/UserDetails/UserDetails.tsx | 40 ++++---- .../teleport/src/Users/UserList/UserList.tsx | 11 +++ .../teleport/src/Users/Users.story.tsx | 46 +++++----- .../teleport/src/services/user/makeUser.ts | 16 +++- .../teleport/src/services/user/types.ts | 4 + .../teleport/src/services/user/user.test.ts | 41 ++++++++- 11 files changed, 346 insertions(+), 94 deletions(-) create mode 100644 web/packages/teleport/src/Users/UserDetails/UserDetails.test.tsx diff --git a/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx b/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx index 6fa9747b1a0..8373225584e 100644 --- a/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx +++ b/web/packages/shared/components/UserDisplayName/UserDisplayName.story.tsx @@ -17,16 +17,16 @@ */ import type { Meta, StoryObj } from '@storybook/react-vite'; -import type { ReactNode } from 'react'; +import type { ComponentProps, ReactNode } from 'react'; import styled from 'styled-components'; import { Flex, Text } from 'design'; import { UserDisplayName } from './UserDisplayName'; -const meta: Meta = { +const meta = { title: 'Shared/UserDisplayName', - component: UserDisplayName, + component: Wrapper, args: { username: 'alice@example.com', primaryText: 'Alice Jones', @@ -46,11 +46,15 @@ const meta: Meta = { ), ], -}; +} satisfies Meta; export default meta; -type Story = StoryObj; +type Story = StoryObj; + +function Wrapper(props: ComponentProps) { + return ; +} export const Playground: Story = {}; @@ -64,7 +68,7 @@ export const LayoutVariants: Story = { > { } } - it('formats inline username with delimiters', () => { + it('formats inline supporting values with delimiters', () => { render( { const primaryLine = screen.getByText('Alice Jones') .parentElement as HTMLElement; - expect(within(primaryLine).getByText('Engineering')).toBeInTheDocument(); const inlineUsername = within(primaryLine).getByText(username); - expect(inlineUsername).toHaveStyleRule('content', "'('", { + const inlineSupportingValues = inlineUsername.parentElement as HTMLElement; + const inlineSecondary = within(inlineSupportingValues).getByText( + 'Engineering' + ); + + expect(primaryLine).toContainElement(inlineSupportingValues); + expect(inlineSupportingValues).toHaveStyleRule('content', "'('", { modifier: '::before', }); - expect(inlineUsername).toHaveStyleRule('content', "')'", { + expect(inlineSupportingValues).toHaveStyleRule('content', "')'", { modifier: '::after', }); + expect(inlineSecondary).toHaveStyleRule('content', "'•'", { + modifier: '::before', + }); }); - it('renders stacked supporting values outside the primary line', () => { + it('renders stacked supporting values together below the primary line', () => { render( { within(primaryLine).queryByText('Engineering') ).not.toBeInTheDocument(); expect(within(primaryLine).queryByText(username)).not.toBeInTheDocument(); + + const supportingLine = screen.getByText(username) + .parentElement as HTMLElement; + const secondary = within(supportingLine).getByText('Engineering'); + + expect(supportingLine).toContainElement(screen.getByText(username)); + expect(secondary).toHaveStyleRule('content', "'•'", { + modifier: '::before', + }); + }); + + it('does not repeat the username when primary text is absent', () => { + render( + + ); + + expect(screen.queryAllByText(username)).toHaveLength(1); + + const primaryLine = screen.getByText(username).parentElement as HTMLElement; + expect( + within(primaryLine).queryByText('Engineering') + ).not.toBeInTheDocument(); expect(screen.getByText('Engineering')).toBeInTheDocument(); - expect(screen.getByText(username)).toBeInTheDocument(); }); it('defaults to tooltip layout', async () => { @@ -173,6 +206,22 @@ describe('UserDisplayName', () => { expect(within(tooltip).getByText(username)).toBeInTheDocument(); }); + it('anchors tooltip layout to the primary text', () => { + render( + + ); + + const tooltipTrigger = screen.getByLabelText( + 'Alice Jones, Engineering, username alice@example.com' + ); + expect(tooltipTrigger).toBe(screen.getByText('Alice Jones')); + }); + function getTooltipAriaLabel( primary: string, secondary: string | null | undefined, diff --git a/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx b/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx index 7567c7ca0a5..2d3b7115a07 100644 --- a/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx +++ b/web/packages/shared/components/UserDisplayName/UserDisplayName.tsx @@ -39,29 +39,46 @@ export function UserDisplayName({ const displayPrimary = normalizeText(primaryText); const displaySecondary = normalizeText(secondaryText); const primary = displayPrimary || username; + const tooltipLabel = getTooltipAriaLabel(primary, displaySecondary, username); - const primaryValue = {primary}; + const primaryValue = (ariaLabel?: string) => ( + + {primary} + + ); const secondaryValue = displaySecondary && ( {displaySecondary} ); - const usernameValue = - displayPrimary && - (layout === 'inline' ? ( - {username} - ) : ( + const separatedSecondaryValue = displaySecondary && ( + + {displaySecondary} + + ); + + const supportingValues = displayPrimary ? ( + <> {username} - )); + {separatedSecondaryValue} + + ) : ( + secondaryValue + ); switch (layout) { case 'inline': return ( - {primaryValue} - {secondaryValue} - {usernameValue} + {primaryValue()} + {displayPrimary ? ( + + {supportingValues} + + ) : ( + supportingValues + )} ); @@ -69,9 +86,12 @@ export function UserDisplayName({ case 'stacked': return ( - {primaryValue} - {secondaryValue} - {usernameValue} + {primaryValue()} + {displayPrimary ? ( + {supportingValues} + ) : ( + supportingValues + )} ); @@ -79,19 +99,13 @@ export function UserDisplayName({ return ( {displayPrimary ? ( - - - {primaryValue} - - + + + {primaryValue(tooltipLabel)} + + ) : ( - {primaryValue} + {primaryValue()} )} {secondaryValue} @@ -150,6 +164,12 @@ const DisplayLine = styled.span` gap: ${props => props.theme.space[1]}px; `; +const SupportingLine = styled.span` + ${containedContent} + display: inline-flex; + align-items: baseline; +`; + const PrimaryValue = styled(singleLineText).attrs({ typography: 'body2', })``; @@ -164,11 +184,23 @@ const SecondaryValue = styled(singleLineText).attrs({ typography: 'body3', })``; -// The parentheses are decorative wrappers around the inline username — using -// `::before/::after` keeps them out of the React text content so they don't -// appear in `textContent`, snapshots, or the accessibility tree, and lets us -// style them independently from the value itself. -const InlineUsernameValue = styled(UsernameValue)` +const SeparatedSecondaryValue = styled(SecondaryValue)` + &::before { + content: '•'; + margin: 0 ${props => props.theme.space[1]}px; + } +`; + +// Decorative delimiters stay out of textContent +const InlineSupportingValues = styled(Text).attrs({ + as: 'span', + color: 'text.muted', + typography: 'body3', +})` + ${containedContent} + display: inline-flex; + align-items: baseline; + &::before { content: '('; } diff --git a/web/packages/teleport/src/Users/UserDetails/UserDetails.story.tsx b/web/packages/teleport/src/Users/UserDetails/UserDetails.story.tsx index fe5f487e5d2..39ec1f92d6f 100644 --- a/web/packages/teleport/src/Users/UserDetails/UserDetails.story.tsx +++ b/web/packages/teleport/src/Users/UserDetails/UserDetails.story.tsx @@ -38,6 +38,8 @@ export type UserDetailsStoryProps = { userType: UserDetailsAuthType; isBot: boolean; userName: string; + displayPrimary?: string; + displaySecondary?: string; rolesCount: number; traitsCount: number; }; @@ -83,6 +85,12 @@ const meta: Meta = { userName: { control: { type: 'text' }, }, + displayPrimary: { + control: { type: 'text' }, + }, + displaySecondary: { + control: { type: 'text' }, + }, rolesCount: { control: { type: 'select' }, options: [0, 5, 16, 128], @@ -96,6 +104,8 @@ const meta: Meta = { userType: 'local' as const, isBot: false, userName: 'john.the.user', + displayPrimary: 'John The User', + displaySecondary: 'john.the.user@example.com', rolesCount: 16, traitsCount: 5, }, @@ -227,6 +237,8 @@ export function createMockUser(props: UserDetailsStoryProps): User { return { name: props.userName, + displayPrimary: props.displayPrimary, + displaySecondary: props.displaySecondary, authType: config.authType, origin: config.origin, isBot: props.isBot, diff --git a/web/packages/teleport/src/Users/UserDetails/UserDetails.test.tsx b/web/packages/teleport/src/Users/UserDetails/UserDetails.test.tsx new file mode 100644 index 00000000000..30e61238629 --- /dev/null +++ b/web/packages/teleport/src/Users/UserDetails/UserDetails.test.tsx @@ -0,0 +1,91 @@ +/** + * Teleport + * Copyright (C) 2026 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 { render, screen, within } from 'design/utils/testing'; + +import { User } from 'teleport/services/user'; + +import { UserDetails, UserDetailsTitle } from './UserDetails'; + +jest.mock('teleport/lib/locks/useResourceLock', () => ({ + useResourceLock: () => ({ isLocked: false, isLoading: false }), +})); + +describe('UserDetails display names', () => { + it('renders display values in the title and keeps the username field canonical', () => { + const user: User = { + name: 'alice', + roles: ['access'], + authType: 'local', + isLocal: true, + displayPrimary: 'Alice Jones', + displaySecondary: 'alice@example.com', + }; + + const { unmount } = render(); + + expect(screen.getByText('Alice Jones')).toBeInTheDocument(); + expect(screen.getByText('alice')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + + unmount(); + + render(); + + const usernameField = screen.getByText('Username') + .parentElement as HTMLElement; + expect(within(usernameField).getByText('alice')).toBeInTheDocument(); + // Auth Type is in the body, with the resource icon in front. + const authTypeField = screen.getByText('Auth Type') + .parentElement as HTMLElement; + expect(within(authTypeField).getByText('local')).toBeInTheDocument(); + expect( + within(authTypeField).getByTestId('res-icon-server') + ).toBeInTheDocument(); + expect(screen.getByText('Status')).toBeInTheDocument(); + }); + + it('renders secondary-only users with username as the primary value', () => { + const user: User = { + name: 'alice', + roles: ['access'], + authType: 'local', + isLocal: true, + displaySecondary: 'alice@example.com', + }; + + render(); + + expect(screen.getByText('alice')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + }); + + it('renders username-only users without duplicating the username', () => { + const user: User = { + name: 'bot-user', + roles: ['bot-user'], + authType: 'local', + isLocal: true, + isBot: true, + }; + + render(); + + expect(screen.queryAllByText('bot-user')).toHaveLength(1); + }); +}); diff --git a/web/packages/teleport/src/Users/UserDetails/UserDetails.tsx b/web/packages/teleport/src/Users/UserDetails/UserDetails.tsx index ccad08652fa..0d2088c88ff 100644 --- a/web/packages/teleport/src/Users/UserDetails/UserDetails.tsx +++ b/web/packages/teleport/src/Users/UserDetails/UserDetails.tsx @@ -28,6 +28,7 @@ import { InfoParagraph, InfoTitle, } from 'shared/components/SlidingSidePanel/InfoGuide'; +import { UserDisplayName } from 'shared/components/UserDisplayName'; import cfg from 'teleport/config'; import { useResourceLock } from 'teleport/lib/locks/useResourceLock'; @@ -86,9 +87,19 @@ export function UserDetails({ Auth Type - - {renderAuthType(user).text} - + + + + {renderAuthType(user).text} + + Status @@ -196,8 +207,6 @@ export function UserDetailsTitle({ onDelete, panelWidth = 480, }: UserDetailsTitleProps) { - const { text: authType, icon } = renderAuthType(user); - // needed to fill InfoGuidePanel for UserDetailsActions const containerWidth = panelWidth - 80; const userIconSize = 48; @@ -215,21 +224,12 @@ export function UserDetailsTitle({ )} - - {user.name} - - - - - {authType} - {user.isBot ? ' (Bot)' : ''} - - + ( + + + + ), }, { key: 'roles', diff --git a/web/packages/teleport/src/Users/Users.story.tsx b/web/packages/teleport/src/Users/Users.story.tsx index e12be8bdfd8..b86fc352fd4 100644 --- a/web/packages/teleport/src/Users/Users.story.tsx +++ b/web/packages/teleport/src/Users/Users.story.tsx @@ -19,6 +19,7 @@ import type { StoryObj } from '@storybook/react-vite'; import { delay } from 'msw'; +import { ContentMinWidth } from 'teleport/Main/Main'; import { TeleportProviderBasic } from 'teleport/mocks/providers'; import { errorGetUsers, @@ -35,14 +36,17 @@ export default { const users = [ { name: 'cikar@egaposci.me', + displayPrimary: 'Cikar Egaposci', + displaySecondary: 'cikar@egaposci.me', roles: ['admin'], - authType: 'teleport local user', + authType: 'local user', isLocal: true, }, { name: 'hi@nen.pa', + displaySecondary: 'hi@nen.pa', roles: ['ruhh', 'admin'], - authType: 'teleport local user', + authType: 'local user', isLocal: true, }, { @@ -66,13 +70,13 @@ const users = [ { name: 'azesotil@jevig.org', roles: ['tugu'], - authType: 'teleport local user', + authType: 'local user', isLocal: true, }, { name: 'bot-little-robot', roles: ['bot-little-robot'], - authType: 'teleport local user', + authType: 'local user', isLocal: true, isBot: true, }, @@ -85,11 +89,7 @@ export const Loaded: StoryObj = { }, }, render() { - return ( - - - - ); + return renderUsers(sample); }, }; @@ -100,11 +100,7 @@ export const UsersNotEqualMauNotice: StoryObj = { }, }, render() { - return ( - - - - ); + return renderUsers({ ...sample, showMauInfo: true }); }, }; @@ -115,11 +111,7 @@ export const Processing: StoryObj = { }, }, render() { - return ( - - - - ); + return renderUsers(sample); }, }; @@ -130,14 +122,20 @@ export const Failed: StoryObj = { }, }, render() { - return ( - - - - ); + return renderUsers(sample); }, }; +function renderUsers(props) { + return ( + + + + + + ); +} + const sample = { attempt: { isProcessing: false, diff --git a/web/packages/teleport/src/services/user/makeUser.ts b/web/packages/teleport/src/services/user/makeUser.ts index d700ed7f4f3..64963aee2f5 100644 --- a/web/packages/teleport/src/services/user/makeUser.ts +++ b/web/packages/teleport/src/services/user/makeUser.ts @@ -20,12 +20,24 @@ import { User } from './types'; export default function makeUser(json: any): User { json = json || {}; - const { name, roles, authType, origin, traits = {}, allTraits, isBot } = json; + const { + name, + roles, + authType, + origin, + traits = {}, + allTraits, + isBot, + displayPrimary, + displaySecondary, + } = json; return { name, + displayPrimary, + displaySecondary, roles: roles ? roles.sort() : [], - authType: authType === 'local' ? 'teleport local user' : authType, + authType: authType === 'local' ? 'local user' : authType, isLocal: authType === 'local', isBot, origin: origin ? origin : '', diff --git a/web/packages/teleport/src/services/user/types.ts b/web/packages/teleport/src/services/user/types.ts index e6329139a99..0612003e0c8 100644 --- a/web/packages/teleport/src/services/user/types.ts +++ b/web/packages/teleport/src/services/user/types.ts @@ -140,6 +140,10 @@ export type UserOrigin = 'okta' | 'saml' | 'scim'; export interface User { // name is the teleport username. name: string; + // displayPrimary is the human-readable name resolved server-side. + displayPrimary?: string; + // displaySecondary is supporting display context resolved server-side. + displaySecondary?: string; // roles is the list of roles user is assigned to. roles: string[]; // authType describes how the user authenticated diff --git a/web/packages/teleport/src/services/user/user.test.ts b/web/packages/teleport/src/services/user/user.test.ts index ef70938cf10..cd909184262 100644 --- a/web/packages/teleport/src/services/user/user.test.ts +++ b/web/packages/teleport/src/services/user/user.test.ts @@ -19,7 +19,7 @@ import cfg from 'teleport/config'; import api from 'teleport/services/api'; -import { makeTraits } from './makeUser'; +import makeUser, { makeTraits } from './makeUser'; import { Acl, ExcludeUserField, PasswordState, User } from './types'; import user from './user'; @@ -428,6 +428,8 @@ test('fetch users, null response values gives empty array', async () => { expect(response).toStrictEqual([ { authType: '', + displayPrimary: undefined, + displaySecondary: undefined, isBot: undefined, isLocal: false, name: '', @@ -447,6 +449,43 @@ test('fetch users, null response values gives empty array', async () => { ]); }); +test('makeUser maps display name fields when present', () => { + expect( + makeUser({ + name: 'alice', + roles: ['access'], + displayPrimary: 'Alice Jones', + displaySecondary: 'alice@example.com', + }) + ).toMatchObject({ + name: 'alice', + displayPrimary: 'Alice Jones', + displaySecondary: 'alice@example.com', + }); + + expect(makeUser({ name: 'bob', roles: [] })).toMatchObject({ + displayPrimary: undefined, + displaySecondary: undefined, + }); +}); + +test('makeUser labels local users as "local user"', () => { + expect( + makeUser({ name: 'alice', roles: [], authType: 'local' }) + ).toMatchObject({ + authType: 'local user', + isLocal: true, + }); + + // Non-local auth types pass through unchanged. + expect( + makeUser({ name: 'bob', roles: [], authType: 'github' }) + ).toMatchObject({ + authType: 'github', + isLocal: false, + }); +}); + test('createResetPasswordToken', async () => { // Test null response. jest.spyOn(api, 'post').mockResolvedValue(null);