Skip to content

Commit

Permalink
Disable file transfer buttons if user lacks permissions (#49892)
Browse files Browse the repository at this point in the history
This will disable the upload/download buttons in the web terminal if the
user does not have the role option `ssh_file_copy`
  • Loading branch information
avatus authored Dec 6, 2024
1 parent 8f4e8c9 commit eea8292
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 19 deletions.
4 changes: 4 additions & 0 deletions lib/services/useracl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -262,5 +265,6 @@ func NewUserACL(user types.User, userRoles RoleSet, features proto.Features, des
CrownJewel: crownJewelAccess,
AccessGraphSettings: accessGraphSettings,
Contact: contact,
FileTransferAccess: fileTransferAccess,
}
}
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*/

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(
<FileTransferContextProvider>
<FileTransferActionBar hasAccess={true} isConnected={true} />
</FileTransferContextProvider>
);

expect(screen.getByTitle('Download files')).toBeEnabled();
expect(screen.getByTitle('Upload files')).toBeEnabled();
});

test('file transfer is disable if no access', async () => {
render(
<FileTransferContextProvider>
<FileTransferActionBar hasAccess={false} isConnected={true} />
</FileTransferContextProvider>
);

expect(screen.getByTitle('Download files')).toBeDisabled();
expect(screen.getByTitle('Upload files')).toBeDisabled();
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Flex flex="none" alignItems="center" height="24px">
<ButtonIcon
disabled={areFileTransferButtonsDisabled}
size={0}
title="Download files"
onClick={fileTransferContext.openDownloadDialog}
<HoverTooltip
position="bottom"
tipContent={
!hasAccess ? (
<Text>
You are missing the{' '}
<Text bold as="span">
ssh_file_copy
</Text>{' '}
role option.
</Text>
) : null
}
>
<Icons.Download size={16} />
</ButtonIcon>
<ButtonIcon
disabled={areFileTransferButtonsDisabled}
size={0}
title="Upload files"
onClick={fileTransferContext.openUploadDialog}
>
<Icons.Upload size={16} />
</ButtonIcon>
<ButtonIcon
disabled={areFileTransferButtonsDisabled}
size={0}
title="Download files"
onClick={fileTransferContext.openDownloadDialog}
>
<Icons.Download size={16} />
</ButtonIcon>
<ButtonIcon
disabled={areFileTransferButtonsDisabled}
size={0}
title="Upload files"
onClick={fileTransferContext.openUploadDialog}
>
<Icons.Upload size={16} />
</ButtonIcon>
</HoverTooltip>
</Flex>
);
}
68 changes: 68 additions & 0 deletions web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*/

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(<Component ctx={ctx} />);
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(<Component ctx={ctx} />);
expect(screen.getByTitle('Download files')).toBeEnabled();
});

function Component({ ctx }: { ctx: ConsoleContext }) {
return (
<MemoryRouter>
<ConsoleContextProvider value={ctx}>
<DocumentSsh
doc={{
id: 123,
status: 'connected',
kind: 'terminal',
serverId: '123',
login: 'tester',
latency: { client: 123, server: 2 },
url: 'http://localhost',
created: new Date(),
}}
visible={true}
/>
</ConsoleContextProvider>
</MemoryRouter>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<TerminalRef>();
const { tty, status, closeDocument, session } = useSshSession(doc);
const [showSearch, setShowSearch] = useState(false);
Expand Down Expand Up @@ -130,7 +134,10 @@ function DocumentSsh({ doc, visible }: PropTypes) {

return (
<Document visible={visible} flexDirection="column">
<FileTransferActionBar isConnected={doc.status === 'connected'} />
<FileTransferActionBar
hasAccess={hasFileTransferAccess}
isConnected={doc.status === 'connected'}
/>
{status === 'loading' && (
<Box textAlign="center" m={10}>
<Indicator />
Expand Down
1 change: 1 addition & 0 deletions web/packages/teleport/src/mocks/contexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export const allAccessAcl: Acl = {
desktopSessionRecordingEnabled: true,
directorySharingEnabled: true,
reviewRequests: true,
fileTransferAccess: true,
license: fullAccess,
download: fullAccess,
plugins: fullAccess,
Expand Down
6 changes: 6 additions & 0 deletions web/packages/teleport/src/services/user/makeAcl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -115,6 +120,7 @@ export function makeAcl(json): Acl {
accessMonitoringRule,
discoverConfigs,
contacts,
fileTransferAccess,
};
}

Expand Down
1 change: 1 addition & 0 deletions web/packages/teleport/src/services/user/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export interface Acl {
bots: Access;
accessMonitoringRule: Access;
contacts: Access;
fileTransferAccess: boolean;
}

// AllTraits represent all the traits defined for a user.
Expand Down
1 change: 1 addition & 0 deletions web/packages/teleport/src/services/user/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 4 additions & 0 deletions web/packages/teleport/src/stores/storeUserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ export default class StoreUserContext extends Store<UserContext> {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,12 @@ export function DocumentTerminal(props: {
autoFocusDisabled={true}
>
<FileTransferContextProvider>
<FileTransferActionBar isConnected={docConnected} />
<FileTransferActionBar
hasAccess={
true /* TODO (avatus) use `fileTransferAccess` ACL property when it gets added */
}
isConnected={docConnected}
/>
{attempt.status === 'success' && (
<Terminal
// The key prop makes sure that we render Terminal only once for each PTY process.
Expand Down

0 comments on commit eea8292

Please sign in to comment.