Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Add warning dialog for unsupported browsers for directory sharing #1110

Merged
ibeckermayer merged 4 commits into from
Aug 15, 2022
Merged

Add warning dialog for unsupported browsers for directory sharing #1110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51d87fc
Adds logic for popping up a non-fatal dialog when the user attempts to
Aug 13, 2022
0d32545
Update packages/teleport/src/DesktopSession/DesktopSession.tsx
Aug 15, 2022
59af539
consolidates directorySharingState into a single object
Aug 15, 2022
7cf6144
Merge branch 'master' into isaiah/sd-handle-unsupported-browsers
Aug 15, 2022
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
16 changes: 12 additions & 4 deletions packages/teleport/src/DesktopSession/DesktopSession.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ const props: State = {
disconnected: false,
setDisconnected: () => {},
setClipboardState: () => {},
canShareDirectory: true,
isSharingDirectory: false,
setIsSharingDirectory: () => {},
directorySharingState: {
canShare: true,
isSharing: false,
browserError: false,
},
setDirectorySharingState: () => {},
onPngFrame: () => {},
onTdpError: () => {},
onKeyDown: () => {},
Expand All @@ -79,6 +82,7 @@ const props: State = {
authenticate: () => {},
setState: () => {},
},
isUsingChrome: true,
};

export const Processing = () => (
Expand Down Expand Up @@ -141,7 +145,11 @@ export const ConnectedSettingsTrue = () => {
permission: { state: 'granted' },
errorText: '',
}}
isSharingDirectory={true}
directorySharingState={{
canShare: true,
isSharing: true,
browserError: false,
}}
onPngFrame={(ctx: CanvasRenderingContext2D) => {
fillGray(ctx.canvas);
}}
Expand Down
76 changes: 52 additions & 24 deletions packages/teleport/src/DesktopSession/DesktopSession.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ declare global {

export function DesktopSession(props: State) {
const {
directorySharingState,
setDirectorySharingState,
clipboardState,
fetchAttempt,
tdpConnection,
Expand All @@ -62,21 +64,23 @@ export function DesktopSession(props: State) {
// onDialogClose is called when a user
// dismisses a non-fatal error dialog.
const onDialogClose = () => {
// This setTdpConnection call will cause the useEffect below
// to calculate the errorDialog state.
// The following state-setting calls will
// cause the useEffect below to calculate the
// errorDialog state.

setTdpConnection(prevState => {
// onDialogClose should only be called when
// the user dismisses a non-fatal error dialog,
// and prevState.status === '' means non-fatal
// error dialog, so the below if statement should
// always be true.
if (prevState.status === '') {
// If prevState.status was a non-fatal error,
// we assume that the TDP connection remains open.
return { status: 'success' };
}
return prevState;
});

setDirectorySharingState(prevState => ({
...prevState,
browserError: false,
}));
};

const computeErrorDialog = () => {
Expand All @@ -100,9 +104,18 @@ export function DesktopSession(props: State) {
errorText = clipboardState.errorText || 'clipboard sharing failed';
} else if (unknownConnectionError) {
errorText = 'Session disconnected for an unknown reason.';
} else if (directorySharingState.browserError) {
errorText =
'Your user role supports directory sharing over desktop access, \
however this feature is only available by default on some Chromium \
based browsers like Google Chrome or Microsoft Edge. Brave users can \
use the feature by navigating to brave://flags/#file-system-access-api \
and selecting "Enable". Please switch to a supported browser.';
}
const open = errorText !== '';
const fatal = tdpConnection.status !== '';
const fatal = !(
tdpConnection.status === '' || directorySharingState.browserError
);

return { open, text: errorText, fatal };
};
Expand Down Expand Up @@ -194,9 +207,8 @@ function Session(props: PropsWithChildren<State>) {
hostname,
clipboardState,
setClipboardState,
canShareDirectory,
isSharingDirectory,
setIsSharingDirectory,
directorySharingState,
setDirectorySharingState,
onPngFrame,
onClipboardData,
onTdpError,
Expand Down Expand Up @@ -229,16 +241,29 @@ function Session(props: PropsWithChildren<State>) {
clipboardSuccess;

const onShareDirectory = () => {
window
.showDirectoryPicker()
.then(sharedDirHandle => {
setIsSharingDirectory(true);
tdpClient.addSharedDirectory(sharedDirHandle);
tdpClient.sendSharedDirectoryAnnounce();
})
.catch(() => {
setIsSharingDirectory(false);
});
try {
window
.showDirectoryPicker()
.then(sharedDirHandle => {
setDirectorySharingState(prevState => ({
...prevState,
isSharing: true,
}));
tdpClient.addSharedDirectory(sharedDirHandle);
tdpClient.sendSharedDirectoryAnnounce();
})
.catch(() => {
setDirectorySharingState(prevState => ({
...prevState,
isSharing: false,
}));
});
} catch (e) {
Copy link
Copy Markdown
Contributor

@zmb3 zmb3 Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not used to seeing a promise's .catch used in conjunction with a standard catch block. Are both of them going to be called in the event of an error?

Copy link
Copy Markdown
Author

@ibeckermayer ibeckermayer Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both of them going to be called in the event of an error?

No, what this means is if the Promise returned by window.showDirectoryPicker() is rejected (which happens if the user closes the directory picker without selecting a directory), we setIsSharingDirectory(false);. On the other hand, if the call to window.showDirectoryPicker() throws an error (as happens on browsers where this feature is not implemented, i.e.:Uncaught TypeError: window.showDirectoryPicker is not a function), we setDirectorySharingBrowserErr(true);.

Granted I'm making the assumption that those are the only two situations common enough to distinguish between, and we don't need to implement any finer grained handling within each block. In theory it's possible that there's some other reason that the promise might be rejected, and some other error that could be thrown, in which case the behavior may or may not be what we want.

setDirectorySharingState(prevState => ({
...prevState,
browserError: true,
}));
}
};

return (
Expand All @@ -250,13 +275,16 @@ function Session(props: PropsWithChildren<State>) {
...prevState,
enabled: false,
}));
setIsSharingDirectory(false);
setDirectorySharingState(prevState => ({
...prevState,
isSharing: false,
}));
tdpClient.nuke();
}}
userHost={`${username}@${hostname}`}
clipboardSharingEnabled={clipboardSharingActive}
canShareDirectory={canShareDirectory}
isSharingDirectory={isSharingDirectory}
canShareDirectory={directorySharingState.canShare}
isSharingDirectory={directorySharingState.isSharing}
onShareDirectory={onShareDirectory}
/>

Expand Down
20 changes: 13 additions & 7 deletions packages/teleport/src/DesktopSession/useDesktopSession.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ export default function useDesktopSession() {
// disconnected tracks whether the user intentionally disconnected the client
const [disconnected, setDisconnected] = useState(false);

const [canShareDirectory, setCanShareDirectory] = useState(false);
const [isSharingDirectory, setIsSharingDirectory] = useState(false);
const [directorySharingState, setDirectorySharingState] = useState({
canShare: false,
isSharing: false,
browserError: false,
});

const { username, desktopName, clusterId } = useParams<UrlDesktopParams>();

Expand Down Expand Up @@ -126,7 +129,10 @@ export default function useDesktopSession() {
.then(desktop => setHostname(desktop.name)),
userService.fetchUserContext().then(user => {
setHasClipboardSharingEnabled(user.acl.clipboardSharingEnabled);
setCanShareDirectory(user.acl.directorySharingEnabled);
setDirectorySharingState(prevState => ({
...prevState,
canShare: user.acl.directorySharingEnabled,
}));
}),
])
);
Expand All @@ -139,7 +145,7 @@ export default function useDesktopSession() {
setTdpConnection,
setWsConnection,
setClipboardState,
setIsSharingDirectory,
setDirectorySharingState,
enableClipboardSharing:
clipboardState.enabled &&
clipboardState.permission.state === 'granted' &&
Expand All @@ -153,9 +159,9 @@ export default function useDesktopSession() {
username,
clipboardState,
setClipboardState,
canShareDirectory,
isSharingDirectory,
setIsSharingDirectory,
directorySharingState,
setDirectorySharingState,
isUsingChrome,
fetchAttempt,
tdpConnection,
wsConnection,
Expand Down
15 changes: 12 additions & 3 deletions packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function useTdpClientCanvas(props: Props) {
setTdpConnection,
setWsConnection,
setClipboardState,
setIsSharingDirectory,
setDirectorySharingState,
enableClipboardSharing,
} = props;
const [tdpClient, setTdpClient] = useState<TdpClient | null>(null);
Expand Down Expand Up @@ -82,7 +82,10 @@ export default function useTdpClientCanvas(props: Props) {
// Default TdpClientEvent.TDP_ERROR and TdpClientEvent.CLIENT_ERROR handler
const onTdpError = (error: { err: Error; isFatal: boolean }) => {
const { err, isFatal } = error;
setIsSharingDirectory(false);
setDirectorySharingState(prevState => ({
...prevState,
isSharing: false,
}));
setClipboardState(prevState => ({
...prevState,
enabled: false,
Expand Down Expand Up @@ -221,6 +224,12 @@ type Props = {
errorText: string;
}>
>;
setIsSharingDirectory: Dispatch<SetStateAction<boolean>>;
setDirectorySharingState: Dispatch<
SetStateAction<{
canShare: boolean;
isSharing: boolean;
browserError: boolean;
}>
>;
enableClipboardSharing: boolean;
};