Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] Disable file transfer buttons if user lacks permissions #49947

Merged
merged 1 commit into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 '../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}
>
<Icons.Download size={16} />
</ButtonIcon>
<ButtonIcon
disabled={areFileTransferButtonsDisabled}
size={0}
title="Upload files"
onClick={fileTransferContext.openUploadDialog}
<HoverTooltip
tipContent={
!hasAccess ? (
<Text>
You are missing the{' '}
<Text bold as="span">
ssh_file_copy
</Text>{' '}
role option.
</Text>
) : null
}
>
<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 useWebAuthn from 'teleport/lib/useWebAuthn';

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 @@ -112,6 +117,7 @@ export function makeAcl(json): Acl {
bots,
accessMonitoringRule,
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 @@ -106,6 +106,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 @@ -281,6 +281,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
Loading