From d989f802d537023732e4d59fceca6cb731220bb6 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:35:59 -0400 Subject: [PATCH] Disable TTL/grace period editing for server-derived attributes (#38001) * Disable TTL/grace period editing for server-derived attributes * FIx lint * Fix bug with dot menu --- .../session_attributes.scss | 2 + .../session_attributes.test.tsx | 16 ++-- .../session_attributes_dot_menu.test.tsx | 18 +++- .../session_attributes_dot_menu.tsx | 85 ++++++++++--------- .../session_attributes_table.test.tsx | 17 +++- .../session_attributes_table.tsx | 32 ++++--- webapp/channels/src/i18n/en.json | 1 + 7 files changed, 111 insertions(+), 60 deletions(-) diff --git a/webapp/channels/src/components/admin_console/session_attributes/session_attributes.scss b/webapp/channels/src/components/admin_console/session_attributes/session_attributes.scss index 68a70464c380..d8a10dd3f89b 100644 --- a/webapp/channels/src/components/admin_console/session_attributes/session_attributes.scss +++ b/webapp/channels/src/components/admin_console/session_attributes/session_attributes.scss @@ -121,9 +121,11 @@ border-radius: 4px; background: rgba(var(--center-channel-color-rgb), 0.08); color: rgba(var(--center-channel-color-rgb), 0.75); + cursor: pointer; font-size: 10px; font-weight: 600; text-transform: uppercase; + user-select: none; } .SessionAttributes__type-cell { diff --git a/webapp/channels/src/components/admin_console/session_attributes/session_attributes.test.tsx b/webapp/channels/src/components/admin_console/session_attributes/session_attributes.test.tsx index c409b1b9e8aa..907e99829ce6 100644 --- a/webapp/channels/src/components/admin_console/session_attributes/session_attributes.test.tsx +++ b/webapp/channels/src/components/admin_console/session_attributes/session_attributes.test.tsx @@ -59,7 +59,7 @@ function makeField(name: string, type: 'text' | 'select', sortOrder: number, ext } const representativeFields: UserPropertyField[] = [ - makeField('ip_address', 'text', 0, { + makeField('client_ip_address', 'text', 0, { display_name: 'Client IP', platforms: ['desktop', 'browser'], ttl_seconds: 300, @@ -212,9 +212,9 @@ describe('SessionAttributesPage', () => { await screen.findByText('Client IP'); - await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-ip_address')); + await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-client_ip_address')); await userEvent.hover(screen.getByRole('menuitem', {name: /Time-to-live/})); - await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-ip_address-3600')); + await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-client_ip_address-3600')); const saveButton = screen.getByRole('button', {name: /Save/}); expect(saveButton).toBeEnabled(); @@ -228,7 +228,7 @@ describe('SessionAttributesPage', () => { expect(patchPropertyField).toHaveBeenCalledWith( SESSION_ATTRIBUTES_GROUP_ID, SESSION_ATTRIBUTES_OBJECT_TYPE, - 'session-ip_address', + 'session-client_ip_address', {attrs: {ttl_seconds: 3600}}, ); }); @@ -255,9 +255,9 @@ describe('SessionAttributesPage', () => { await screen.findByText('Client IP'); expect(store.getState().views.admin.navigationBlock.blocked).toBe(false); - await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-ip_address')); + await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-client_ip_address')); await userEvent.hover(screen.getByRole('menuitem', {name: /Time-to-live/})); - await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-ip_address-3600')); + await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-client_ip_address-3600')); await waitFor(() => { expect(store.getState().views.admin.navigationBlock.blocked).toBe(true); @@ -277,9 +277,9 @@ describe('SessionAttributesPage', () => { await screen.findByText('Client IP'); - await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-ip_address')); + await userEvent.click(screen.getByTestId('session-attribute-dotmenu-session-client_ip_address')); await userEvent.hover(screen.getByRole('menuitem', {name: /Time-to-live/})); - await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-ip_address-3600')); + await userEvent.click(await screen.findByTestId('session-attribute-ttl-option-session-client_ip_address-3600')); const stagedRow = screen.getAllByText('Client IP')[0].closest('tr') as HTMLElement; expect(within(stagedRow).getByTestId('session-attribute-ttl')).toHaveTextContent('1h'); diff --git a/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.test.tsx b/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.test.tsx index 11316de52133..a6cf8d518cd4 100644 --- a/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.test.tsx +++ b/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.test.tsx @@ -42,7 +42,7 @@ function makeField(name: string, extra: ExtraAttrs = {}): SessionAttributeField } as UserPropertyField; } -const enabledField = makeField('ip_address', { +const enabledField = makeField('client_ip_address', { display_name: 'Client IP', enabled: true, ttl_seconds: 300, @@ -56,6 +56,13 @@ const disabledField = makeField('vpn_active', { grace_period_seconds: 30, }); +const serverSourcedField = makeField('ip_address', { + display_name: 'IP Address', + enabled: true, + ttl_seconds: 300, + grace_period_seconds: 60, +}); + function renderMenu(field: SessionAttributeField, onStageChange = jest.fn()) { renderWithContext(
@@ -125,6 +132,15 @@ describe('SessionAttributesDotMenu', () => { expect(unselected).toHaveAttribute('aria-checked', 'false'); }); + it('hides the TTL and Grace submenus for server-sourced attributes', async () => { + renderMenu(serverSourcedField); + + await userEvent.click(screen.getByTestId(`session-attribute-dotmenu-${serverSourcedField.id}`)); + + expect(screen.queryByRole('menuitem', {name: /Time-to-live/})).not.toBeInTheDocument(); + expect(screen.queryByRole('menuitem', {name: /Grace Period/})).not.toBeInTheDocument(); + }); + it('opens the confirmation modal for Disable without staging immediately', async () => { const onStageChange = renderMenu(enabledField); diff --git a/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.tsx b/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.tsx index fb343fc72c62..3a23a64501b8 100644 --- a/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.tsx +++ b/webapp/channels/src/components/admin_console/session_attributes/session_attributes_dot_menu.tsx @@ -15,7 +15,7 @@ import {ModalIdentifiers} from 'utils/constants'; import DisableAttributeModal from './disable_attribute_modal'; import type {StagedAttrs} from './use_session_attribute_edits'; -import {DURATION_PRESETS_SECONDS, formatDuration, getSessionAttrs, getSessionDisplayName} from './utils'; +import {DURATION_PRESETS_SECONDS, formatDuration, getSessionAttrs, getSessionDisplayName, isServerSourced} from './utils'; import type {SessionAttributeField} from './utils'; type Props = { @@ -30,6 +30,7 @@ export default function SessionAttributesDotMenu({field, onStageChange, disabled const attrs = getSessionAttrs(field); const menuId = `session-attribute-dotmenu-${field.id}`; + const isServerAttribute = isServerSourced(field.name); const handleTtlChange = useCallback((seconds: number) => { onStageChange(field.id, {ttl_seconds: seconds}); @@ -80,45 +81,49 @@ export default function SessionAttributesDotMenu({field, onStageChange, disabled 'aria-label': formatMessage({id: 'admin.session_attributes.dotmenu.menu.aria_label', defaultMessage: 'Select an action'}), }} > - } - labels={( - - )} - trailingElements={( - <> - {formatDuration(attrs.ttl_seconds)} - - - )} - > - {renderPresets(attrs.ttl_seconds, `session-attribute-ttl-option-${field.id}`, handleTtlChange)} - - } - labels={( - - )} - trailingElements={( - <> - {formatDuration(attrs.grace_period_seconds)} - - - )} - > - {renderPresets(attrs.grace_period_seconds, `session-attribute-grace-option-${field.id}`, (seconds) => onStageChange(field.id, {grace_period_seconds: seconds}))} - - + {!isServerAttribute && ([ + } + labels={( + + )} + trailingElements={( + <> + {formatDuration(attrs.ttl_seconds)} + + + )} + > + {renderPresets(attrs.ttl_seconds, `session-attribute-ttl-option-${field.id}`, handleTtlChange)} + , + } + labels={( + + )} + trailingElements={( + <> + {formatDuration(attrs.grace_period_seconds)} + + + )} + > + {renderPresets(attrs.grace_period_seconds, `session-attribute-grace-option-${field.id}`, (seconds) => onStageChange(field.id, {grace_period_seconds: seconds}))} + , + , + ])} {attrs.enabled ? ( { />, ); - const row = rowFor('Client IP'); + const row = rowFor('client_ip_address'); expect(within(row).getByTestId('session-attribute-ttl')).toHaveTextContent('5m'); expect(within(row).getByTestId('session-attribute-grace')).toHaveTextContent('1m'); }); + it('shows a dash for TTL and Grace on server-sourced attributes', () => { + renderWithContext( + , + ); + + const row = rowFor('Client IP'); + expect(within(row).getByTestId('session-attribute-ttl')).toHaveTextContent('—'); + expect(within(row).getByTestId('session-attribute-grace')).toHaveTextContent('—'); + }); + it('falls back to the field name when display_name is absent', () => { renderWithContext( {getValue()} {isServerSourced(row.original.name) && ( - - - + + + + )} ), @@ -154,7 +159,7 @@ export default function SessionAttributesTable({data, onStageChange, disabled = className='SessionAttributes__duration' data-testid='session-attribute-ttl' > - {formatDuration(getSessionAttrs(row.original).ttl_seconds)} + {isServerSourced(row.original.name) ? '—' : formatDuration(getSessionAttrs(row.original).ttl_seconds)} ), enableHiding: false, @@ -176,7 +181,7 @@ export default function SessionAttributesTable({data, onStageChange, disabled = className='SessionAttributes__duration' data-testid='session-attribute-grace' > - {formatDuration(getSessionAttrs(row.original).grace_period_seconds)} + {isServerSourced(row.original.name) ? '—' : formatDuration(getSessionAttrs(row.original).grace_period_seconds)} ), enableHiding: false, @@ -257,3 +262,10 @@ const typeLabels = defineMessages({ Boolean: {id: 'admin.session_attributes.type.boolean', defaultMessage: 'Boolean'}, Enum: {id: 'admin.session_attributes.type.enum', defaultMessage: 'Enum'}, }); + +const messages = defineMessages({ + serverLabelTooltip: { + id: 'admin.session_attributes.table.server_label.tooltip', + defaultMessage: 'This attribute is captured by the server for every request a user makes. It is not delivered by the client.', + }, +}); diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 82c334e72910..5e9cff59bee7 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -3337,6 +3337,7 @@ "admin.session_attributes.table.name": "Name", "admin.session_attributes.table.platform": "Platform", "admin.session_attributes.table.server_label": "Server", + "admin.session_attributes.table.server_label.tooltip": "This attribute is captured by the server for every request a user makes. It is not delivered by the client.", "admin.session_attributes.table.status": "Status", "admin.session_attributes.table.ttl": "TTL", "admin.session_attributes.table.type": "Type",