From 5e20df79e789c335f508ec62d73de7fd1dca8d62 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 9 Dec 2024 12:46:40 -0600 Subject: [PATCH] Disable file transfer buttons if user lacks permissions (#49892) (#49946) This will disable the upload/download buttons in the web terminal if the user does not have the role option `ssh_file_copy` --- lib/services/useracl.go | 4 ++ .../FileTransferActionBar.test.tsx | 44 ++++++++++++ .../FileTransfer/FileTransferActionBar.tsx | 53 ++++++++++----- .../Console/DocumentSsh/DocumentSsh.test.tsx | 68 +++++++++++++++++++ .../src/Console/DocumentSsh/DocumentSsh.tsx | 9 ++- web/packages/teleport/src/mocks/contexts.ts | 1 + .../teleport/src/services/user/makeAcl.ts | 6 ++ .../teleport/src/services/user/types.ts | 1 + .../teleport/src/services/user/user.test.ts | 1 + .../teleport/src/stores/storeUserContext.ts | 4 ++ .../ui/DocumentTerminal/DocumentTerminal.tsx | 7 +- 11 files changed, 179 insertions(+), 19 deletions(-) create mode 100644 web/packages/shared/components/FileTransfer/FileTransferActionBar.test.tsx create mode 100644 web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.test.tsx diff --git a/lib/services/useracl.go b/lib/services/useracl.go index d1511651f8adc..4a6c39b554ee5 100644 --- a/lib/services/useracl.go +++ b/lib/services/useracl.go @@ -118,6 +118,8 @@ type UserACL struct { ReviewRequests bool `json:"reviewRequests"` // Contact defines the ability to manage contacts Contact ResourceAccess `json:"contact"` + // FileTransferAccess defines the ability to perform remote file operations via SCP or SFTP + FileTransferAccess bool `json:"fileTransferAccess"` } func hasAccess(roleSet RoleSet, ctx *Context, kind string, verbs ...string) bool { @@ -210,6 +212,7 @@ func NewUserACL(user types.User, userRoles RoleSet, features proto.Features, des crownJewelAccess := newAccess(userRoles, ctx, types.KindCrownJewel) userTasksAccess := newAccess(userRoles, ctx, types.KindUserTask) reviewRequests := userRoles.MaybeCanReviewRequests() + fileTransferAccess := userRoles.CanCopyFiles() var auditQuery ResourceAccess var securityReports ResourceAccess @@ -262,5 +265,6 @@ func NewUserACL(user types.User, userRoles RoleSet, features proto.Features, des CrownJewel: crownJewelAccess, AccessGraphSettings: accessGraphSettings, Contact: contact, + FileTransferAccess: fileTransferAccess, } } diff --git a/web/packages/shared/components/FileTransfer/FileTransferActionBar.test.tsx b/web/packages/shared/components/FileTransfer/FileTransferActionBar.test.tsx new file mode 100644 index 0000000000000..9f948abeda683 --- /dev/null +++ b/web/packages/shared/components/FileTransfer/FileTransferActionBar.test.tsx @@ -0,0 +1,44 @@ +/** + * Teleport + * Copyright (C) 2024 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 } from 'design/utils/testing'; + +import { FileTransferActionBar } from './FileTransferActionBar'; +import { FileTransferContextProvider } from './FileTransferContextProvider'; + +test('file transfer bar is enabled by default', async () => { + render( + + + + ); + + expect(screen.getByTitle('Download files')).toBeEnabled(); + expect(screen.getByTitle('Upload files')).toBeEnabled(); +}); + +test('file transfer is disable if no access', async () => { + render( + + + + ); + + expect(screen.getByTitle('Download files')).toBeDisabled(); + expect(screen.getByTitle('Upload files')).toBeDisabled(); +}); diff --git a/web/packages/shared/components/FileTransfer/FileTransferActionBar.tsx b/web/packages/shared/components/FileTransfer/FileTransferActionBar.tsx index de894ca2e0d89..74c0616f8add0 100644 --- a/web/packages/shared/components/FileTransfer/FileTransferActionBar.tsx +++ b/web/packages/shared/components/FileTransfer/FileTransferActionBar.tsx @@ -17,40 +17,59 @@ */ import React from 'react'; -import { Flex, ButtonIcon } from 'design'; +import { Flex, ButtonIcon, Text } from 'design'; import * as Icons from 'design/Icon'; +import { HoverTooltip } from 'design/Tooltip'; import { useFileTransferContext } from './FileTransferContextProvider'; type FileTransferActionBarProps = { isConnected: boolean; + // if any role has `options.ssh_file_copy: true` without any other role having `false` (false overrides). + hasAccess: boolean; }; export function FileTransferActionBar({ isConnected, + hasAccess, }: FileTransferActionBarProps) { const fileTransferContext = useFileTransferContext(); const areFileTransferButtonsDisabled = - fileTransferContext.openedDialog || !isConnected; + fileTransferContext.openedDialog || !isConnected || !hasAccess; return ( - + You are missing the{' '} + + ssh_file_copy + {' '} + role option. + + ) : null + } > - - - - - + + + + + + + ); } diff --git a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.test.tsx b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.test.tsx new file mode 100644 index 0000000000000..0003326ef754f --- /dev/null +++ b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.test.tsx @@ -0,0 +1,68 @@ +/** + * Teleport + * Copyright (C) 2024 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 } from 'design/utils/testing'; +import { MemoryRouter } from 'react-router'; +import 'jest-canvas-mock'; + +import { allAccessAcl } from 'teleport/mocks/contexts'; + +import ConsoleContext from '../consoleContext'; +import ConsoleContextProvider from '../consoleContextProvider'; + +import DocumentSsh from '.'; + +test('file transfer buttons are disabled if user does not have access', async () => { + const ctx = new ConsoleContext(); + ctx.storeUser.setState({ + acl: { ...allAccessAcl, fileTransferAccess: false }, + }); + render(); + expect(screen.getByTitle('Download files')).toBeDisabled(); +}); + +test('file transfer buttons are enabled if user has access', async () => { + const ctx = new ConsoleContext(); + ctx.storeUser.setState({ + acl: allAccessAcl, + }); + render(); + expect(screen.getByTitle('Download files')).toBeEnabled(); +}); + +function Component({ ctx }: { ctx: ConsoleContext }) { + return ( + + + + + + ); +} diff --git a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx index 8a81c4ec6031b..01138ab3c70bf 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx +++ b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx @@ -36,6 +36,8 @@ import { useMfa } from 'teleport/lib/useMfa'; import Document from '../Document'; +import { useConsoleContext } from '../consoleContextProvider'; + import { Terminal, TerminalRef } from './Terminal'; import useSshSession from './useSshSession'; import { useFileTransfer } from './useFileTransfer'; @@ -49,6 +51,8 @@ export default function DocumentSshWrapper(props: PropTypes) { } function DocumentSsh({ doc, visible }: PropTypes) { + const ctx = useConsoleContext(); + const hasFileTransferAccess = ctx.storeUser.hasFileTransferAccess(); const terminalRef = useRef(); const { tty, status, closeDocument, session } = useSshSession(doc); const [showSearch, setShowSearch] = useState(false); @@ -130,7 +134,10 @@ function DocumentSsh({ doc, visible }: PropTypes) { return ( - + {status === 'loading' && ( diff --git a/web/packages/teleport/src/mocks/contexts.ts b/web/packages/teleport/src/mocks/contexts.ts index d44f57b5da2f1..2d1378f964fd3 100644 --- a/web/packages/teleport/src/mocks/contexts.ts +++ b/web/packages/teleport/src/mocks/contexts.ts @@ -60,6 +60,7 @@ export const allAccessAcl: Acl = { desktopSessionRecordingEnabled: true, directorySharingEnabled: true, reviewRequests: true, + fileTransferAccess: true, license: fullAccess, download: fullAccess, plugins: fullAccess, diff --git a/web/packages/teleport/src/services/user/makeAcl.ts b/web/packages/teleport/src/services/user/makeAcl.ts index 5fe0a7768cfa5..560add4279e31 100644 --- a/web/packages/teleport/src/services/user/makeAcl.ts +++ b/web/packages/teleport/src/services/user/makeAcl.ts @@ -40,6 +40,11 @@ export function makeAcl(json): Acl { const db = json.db || defaultAccess; const desktops = json.desktops || defaultAccess; const reviewRequests = json.reviewRequests ?? false; + // TODO (avatus) change default to false in v19. We do not want someone + // who _can_ access file transfers to be denied access because an older cluster + // doesn't return the valid permission. If they don't have access, the action will + // still fail with an error, so this is merely a UX improvment. + const fileTransferAccess = json.fileTransferAccess ?? true; // use nullish coalescing to prevent default from overriding a strictly false value const connectionDiagnostic = json.connectionDiagnostic || defaultAccess; // Defaults to true, see RFD 0049 // https://github.com/gravitational/teleport/blob/master/rfd/0049-desktop-clipboard.md#security @@ -115,6 +120,7 @@ export function makeAcl(json): Acl { accessMonitoringRule, discoverConfigs, contacts, + fileTransferAccess, }; } diff --git a/web/packages/teleport/src/services/user/types.ts b/web/packages/teleport/src/services/user/types.ts index 76ea8202d8f35..37239dcd8d34d 100644 --- a/web/packages/teleport/src/services/user/types.ts +++ b/web/packages/teleport/src/services/user/types.ts @@ -108,6 +108,7 @@ export interface Acl { bots: Access; accessMonitoringRule: Access; contacts: Access; + fileTransferAccess: boolean; } // AllTraits represent all the traits defined for a user. diff --git a/web/packages/teleport/src/services/user/user.test.ts b/web/packages/teleport/src/services/user/user.test.ts index d75023f3f7dc4..f734942a9d4fa 100644 --- a/web/packages/teleport/src/services/user/user.test.ts +++ b/web/packages/teleport/src/services/user/user.test.ts @@ -288,6 +288,7 @@ test('undefined values in context response gives proper default values', async ( clipboardSharingEnabled: true, desktopSessionRecordingEnabled: true, directorySharingEnabled: true, + fileTransferAccess: true, }; expect(response).toEqual({ diff --git a/web/packages/teleport/src/stores/storeUserContext.ts b/web/packages/teleport/src/stores/storeUserContext.ts index 878bf68c95405..e50f9fb9d6e61 100644 --- a/web/packages/teleport/src/stores/storeUserContext.ts +++ b/web/packages/teleport/src/stores/storeUserContext.ts @@ -161,6 +161,10 @@ export default class StoreUserContext extends Store { return this.state.acl.samlIdpServiceProvider; } + hasFileTransferAccess() { + return this.state.acl.fileTransferAccess; + } + // hasPrereqAccessToAddAgents checks if user meets the prerequisite // access to add an agent: // - user should be able to create provisioning tokens diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx index 503383cbdb651..def77da4ec252 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx @@ -133,7 +133,12 @@ export function DocumentTerminal(props: { autoFocusDisabled={true} > - + {attempt.status === 'success' && (