From 51d87fc96e75ed8f66df5ebd719610dd92f496bc Mon Sep 17 00:00:00 2001 From: Isaiah Becker-Mayer Date: Fri, 12 Aug 2022 22:41:48 -0700 Subject: [PATCH 1/3] Adds logic for popping up a non-fatal dialog when the user attempts to begin directory sharing on an unsupported browser. --- .../DesktopSession/DesktopSession.story.tsx | 3 ++ .../src/DesktopSession/DesktopSession.tsx | 49 ++++++++++++------- .../src/DesktopSession/useDesktopSession.tsx | 5 ++ 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/packages/teleport/src/DesktopSession/DesktopSession.story.tsx b/packages/teleport/src/DesktopSession/DesktopSession.story.tsx index d35b2f44c..fe5856c6b 100644 --- a/packages/teleport/src/DesktopSession/DesktopSession.story.tsx +++ b/packages/teleport/src/DesktopSession/DesktopSession.story.tsx @@ -79,6 +79,9 @@ const props: State = { authenticate: () => {}, setState: () => {}, }, + directorySharingBrowserErr: false, + setDirectorySharingBrowserErr: () => {}, + isUsingChrome: true, }; export const Processing = () => ( diff --git a/packages/teleport/src/DesktopSession/DesktopSession.tsx b/packages/teleport/src/DesktopSession/DesktopSession.tsx index 38a331c8c..41bf04245 100644 --- a/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -43,6 +43,8 @@ declare global { export function DesktopSession(props: State) { const { + directorySharingBrowserErr, + setDirectorySharingBrowserErr, clipboardState, fetchAttempt, tdpConnection, @@ -62,14 +64,11 @@ 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. @@ -77,6 +76,8 @@ export function DesktopSession(props: State) { } return prevState; }); + + setDirectorySharingBrowserErr(false); }; const computeErrorDialog = () => { @@ -100,9 +101,16 @@ export function DesktopSession(props: State) { errorText = clipboardState.errorText || 'clipboard sharing failed'; } else if (unknownConnectionError) { errorText = 'Session disconnected for an unknown reason.'; + } else if (directorySharingBrowserErr) { + 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 === '' || directorySharingBrowserErr); return { open, text: errorText, fatal }; }; @@ -197,6 +205,7 @@ function Session(props: PropsWithChildren) { canShareDirectory, isSharingDirectory, setIsSharingDirectory, + setDirectorySharingBrowserErr, onPngFrame, onClipboardData, onTdpError, @@ -229,16 +238,20 @@ function Session(props: PropsWithChildren) { clipboardSuccess; const onShareDirectory = () => { - window - .showDirectoryPicker() - .then(sharedDirHandle => { - setIsSharingDirectory(true); - tdpClient.addSharedDirectory(sharedDirHandle); - tdpClient.sendSharedDirectoryAnnounce(); - }) - .catch(() => { - setIsSharingDirectory(false); - }); + try { + window + .showDirectoryPicker() + .then(sharedDirHandle => { + setIsSharingDirectory(true); + tdpClient.addSharedDirectory(sharedDirHandle); + tdpClient.sendSharedDirectoryAnnounce(); + }) + .catch(() => { + setIsSharingDirectory(false); + }); + } catch (e) { + setDirectorySharingBrowserErr(true); + } }; return ( diff --git a/packages/teleport/src/DesktopSession/useDesktopSession.tsx b/packages/teleport/src/DesktopSession/useDesktopSession.tsx index 3119decce..f633a5942 100644 --- a/packages/teleport/src/DesktopSession/useDesktopSession.tsx +++ b/packages/teleport/src/DesktopSession/useDesktopSession.tsx @@ -47,6 +47,8 @@ export default function useDesktopSession() { const [canShareDirectory, setCanShareDirectory] = useState(false); const [isSharingDirectory, setIsSharingDirectory] = useState(false); + const [directorySharingBrowserErr, setDirectorySharingBrowserErr] = + useState(false); const { username, desktopName, clusterId } = useParams(); @@ -155,7 +157,10 @@ export default function useDesktopSession() { setClipboardState, canShareDirectory, isSharingDirectory, + directorySharingBrowserErr, + setDirectorySharingBrowserErr, setIsSharingDirectory, + isUsingChrome, fetchAttempt, tdpConnection, wsConnection, From 0d3254523b95e9ee8358d33e7ebb557dcf0b60eb Mon Sep 17 00:00:00 2001 From: Isaiah Becker-Mayer Date: Mon, 15 Aug 2022 16:31:18 -0400 Subject: [PATCH 2/3] Update packages/teleport/src/DesktopSession/DesktopSession.tsx Co-authored-by: Zac Bergquist --- packages/teleport/src/DesktopSession/DesktopSession.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/teleport/src/DesktopSession/DesktopSession.tsx b/packages/teleport/src/DesktopSession/DesktopSession.tsx index 41bf04245..cdf4986a5 100644 --- a/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -104,7 +104,7 @@ export function DesktopSession(props: State) { } else if (directorySharingBrowserErr) { errorText = 'Your user role supports directory sharing over desktop access, \ - however this feature is only available by default on some chromium \ + 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.'; From 59af539a62d4d09ccc2f16b8280f185d9378d786 Mon Sep 17 00:00:00 2001 From: Isaiah Becker-Mayer Date: Mon, 15 Aug 2022 17:02:16 -0400 Subject: [PATCH 3/3] consolidates directorySharingState into a single object --- .../DesktopSession/DesktopSession.story.tsx | 17 ++++--- .../src/DesktopSession/DesktopSession.tsx | 45 ++++++++++++------- .../src/DesktopSession/useDesktopSession.tsx | 23 +++++----- .../src/DesktopSession/useTdpClientCanvas.tsx | 15 +++++-- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/packages/teleport/src/DesktopSession/DesktopSession.story.tsx b/packages/teleport/src/DesktopSession/DesktopSession.story.tsx index fe5856c6b..ef55c17ff 100644 --- a/packages/teleport/src/DesktopSession/DesktopSession.story.tsx +++ b/packages/teleport/src/DesktopSession/DesktopSession.story.tsx @@ -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: () => {}, @@ -79,8 +82,6 @@ const props: State = { authenticate: () => {}, setState: () => {}, }, - directorySharingBrowserErr: false, - setDirectorySharingBrowserErr: () => {}, isUsingChrome: true, }; @@ -144,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); }} diff --git a/packages/teleport/src/DesktopSession/DesktopSession.tsx b/packages/teleport/src/DesktopSession/DesktopSession.tsx index cdf4986a5..fe824cf1a 100644 --- a/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -43,8 +43,8 @@ declare global { export function DesktopSession(props: State) { const { - directorySharingBrowserErr, - setDirectorySharingBrowserErr, + directorySharingState, + setDirectorySharingState, clipboardState, fetchAttempt, tdpConnection, @@ -77,7 +77,10 @@ export function DesktopSession(props: State) { return prevState; }); - setDirectorySharingBrowserErr(false); + setDirectorySharingState(prevState => ({ + ...prevState, + browserError: false, + })); }; const computeErrorDialog = () => { @@ -101,7 +104,7 @@ export function DesktopSession(props: State) { errorText = clipboardState.errorText || 'clipboard sharing failed'; } else if (unknownConnectionError) { errorText = 'Session disconnected for an unknown reason.'; - } else if (directorySharingBrowserErr) { + } 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 \ @@ -110,7 +113,9 @@ export function DesktopSession(props: State) { and selecting "Enable". Please switch to a supported browser.'; } const open = errorText !== ''; - const fatal = !(tdpConnection.status === '' || directorySharingBrowserErr); + const fatal = !( + tdpConnection.status === '' || directorySharingState.browserError + ); return { open, text: errorText, fatal }; }; @@ -202,10 +207,8 @@ function Session(props: PropsWithChildren) { hostname, clipboardState, setClipboardState, - canShareDirectory, - isSharingDirectory, - setIsSharingDirectory, - setDirectorySharingBrowserErr, + directorySharingState, + setDirectorySharingState, onPngFrame, onClipboardData, onTdpError, @@ -242,15 +245,24 @@ function Session(props: PropsWithChildren) { window .showDirectoryPicker() .then(sharedDirHandle => { - setIsSharingDirectory(true); + setDirectorySharingState(prevState => ({ + ...prevState, + isSharing: true, + })); tdpClient.addSharedDirectory(sharedDirHandle); tdpClient.sendSharedDirectoryAnnounce(); }) .catch(() => { - setIsSharingDirectory(false); + setDirectorySharingState(prevState => ({ + ...prevState, + isSharing: false, + })); }); } catch (e) { - setDirectorySharingBrowserErr(true); + setDirectorySharingState(prevState => ({ + ...prevState, + browserError: true, + })); } }; @@ -263,13 +275,16 @@ function Session(props: PropsWithChildren) { ...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} /> diff --git a/packages/teleport/src/DesktopSession/useDesktopSession.tsx b/packages/teleport/src/DesktopSession/useDesktopSession.tsx index f633a5942..c9f196f4b 100644 --- a/packages/teleport/src/DesktopSession/useDesktopSession.tsx +++ b/packages/teleport/src/DesktopSession/useDesktopSession.tsx @@ -45,10 +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 [directorySharingBrowserErr, setDirectorySharingBrowserErr] = - useState(false); + const [directorySharingState, setDirectorySharingState] = useState({ + canShare: false, + isSharing: false, + browserError: false, + }); const { username, desktopName, clusterId } = useParams(); @@ -128,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, + })); }), ]) ); @@ -141,7 +145,7 @@ export default function useDesktopSession() { setTdpConnection, setWsConnection, setClipboardState, - setIsSharingDirectory, + setDirectorySharingState, enableClipboardSharing: clipboardState.enabled && clipboardState.permission.state === 'granted' && @@ -155,11 +159,8 @@ export default function useDesktopSession() { username, clipboardState, setClipboardState, - canShareDirectory, - isSharingDirectory, - directorySharingBrowserErr, - setDirectorySharingBrowserErr, - setIsSharingDirectory, + directorySharingState, + setDirectorySharingState, isUsingChrome, fetchAttempt, tdpConnection, diff --git a/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx b/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx index d6f8194ec..4acb8d5a7 100644 --- a/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx +++ b/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx @@ -33,7 +33,7 @@ export default function useTdpClientCanvas(props: Props) { setTdpConnection, setWsConnection, setClipboardState, - setIsSharingDirectory, + setDirectorySharingState, enableClipboardSharing, } = props; const [tdpClient, setTdpClient] = useState(null); @@ -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, @@ -221,6 +224,12 @@ type Props = { errorText: string; }> >; - setIsSharingDirectory: Dispatch>; + setDirectorySharingState: Dispatch< + SetStateAction<{ + canShare: boolean; + isSharing: boolean; + browserError: boolean; + }> + >; enableClipboardSharing: boolean; };