From 4182ae3fff49df7335e972e8ca9f459f3a0a0275 Mon Sep 17 00:00:00 2001 From: joerger Date: Sun, 15 Dec 2024 19:05:46 -0800 Subject: [PATCH] Use AuthnDialog for file transfers; Fix json backend logic for file transfers. --- lib/web/files.go | 9 +- .../src/Console/DocumentSsh/DocumentSsh.tsx | 37 ++++---- .../Console/DocumentSsh/useFileTransfer.ts | 88 ++++++++++++++++--- .../src/Console/DocumentSsh/useGetScpUrl.ts | 63 ------------- .../components/AuthnDialog/AuthnDialog.tsx | 40 +++++---- web/packages/teleport/src/lib/useMfa.ts | 3 +- 6 files changed, 128 insertions(+), 112 deletions(-) delete mode 100644 web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts diff --git a/lib/web/files.go b/lib/web/files.go index 62b3913760313..1c48dbf4f745e 100644 --- a/lib/web/files.go +++ b/lib/web/files.go @@ -83,9 +83,12 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou req.mfaResponse = query.Get("webauthn") } - mfaResponse, err := client.ParseMFAChallengeResponse([]byte(req.mfaResponse)) - if err != nil { - return nil, trace.Wrap(err) + var mfaResponse *proto.MFAAuthenticateResponse + if req.mfaResponse != "" { + var err error + if mfaResponse, err = client.ParseMFAChallengeResponse([]byte(req.mfaResponse)); err != nil { + return nil, trace.Wrap(err) + } } // Send an error if only one of these params has been sent. Both should exist or not exist together diff --git a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx index bf12df4ec8797..4ea1b934e076c 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx +++ b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx @@ -16,16 +16,16 @@ * along with this program. If not, see . */ -import React, { useRef, useEffect, useState, useCallback } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { useTheme } from 'styled-components'; -import { Indicator, Box } from 'design'; +import { Box, Indicator } from 'design'; import { - FileTransferActionBar, FileTransfer, - FileTransferRequests, + FileTransferActionBar, FileTransferContextProvider, + FileTransferRequests, } from 'shared/components/FileTransfer'; import { TerminalSearch } from 'shared/components/TerminalSearch'; @@ -39,8 +39,8 @@ import Document from '../Document'; import { useConsoleContext } from '../consoleContextProvider'; import { Terminal, TerminalRef } from './Terminal'; -import useSshSession from './useSshSession'; import { useFileTransfer } from './useFileTransfer'; +import useSshSession from './useSshSession'; export default function DocumentSshWrapper(props: PropTypes) { return ( @@ -57,12 +57,7 @@ function DocumentSsh({ doc, visible }: PropTypes) { const { tty, status, closeDocument, session } = useSshSession(doc); const [showSearch, setShowSearch] = useState(false); const mfa = useMfa(tty); - const { - getMfaResponseAttempt, - getDownloader, - getUploader, - fileTransferRequests, - } = useFileTransfer(tty, session, doc, mfa.mfaRequired); + const ft = useFileTransfer(tty, session, doc, mfa.mfaRequired); const theme = useTheme(); function handleCloseFileTransfer() { @@ -110,21 +105,19 @@ function DocumentSsh({ doc, visible }: PropTypes) { } beforeClose={() => window.confirm('Are you sure you want to cancel file transfers?') } errorText={ - getMfaResponseAttempt.status === 'failed' - ? getMfaResponseAttempt.statusText - : null + ft.mfaAttempt.status === 'error' ? ft.mfaAttempt.statusText : null } afterClose={handleCloseFileTransfer} transferHandlers={{ - getDownloader, - getUploader, + getDownloader: ft.getDownloader, + getUploader: ft.getUploader, }} /> @@ -143,7 +136,15 @@ function DocumentSsh({ doc, visible }: PropTypes) { )} - {mfa.mfaChallenge && } + {mfa.mfaChallenge && } + {ft.mfaChallenge && ( + + )} {status === 'initialized' && terminal} ); diff --git a/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts b/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts index 90c3625a902cf..102e3c5a3eade 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts +++ b/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts @@ -16,18 +16,25 @@ * along with this program. If not, see . */ -import { useEffect, useState, useCallback } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { useFileTransferContext } from 'shared/components/FileTransfer'; -import Tty from 'teleport/lib/term/tty'; +import { DocumentSsh } from 'teleport/Console/stores'; import { EventType } from 'teleport/lib/term/enums'; +import Tty from 'teleport/lib/term/tty'; import { Session } from 'teleport/services/session'; -import { DocumentSsh } from 'teleport/Console/stores'; import { useConsoleContext } from '../consoleContextProvider'; +import { useAsync } from 'shared/hooks/useAsync'; +import cfg from 'teleport/config'; +import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; +import { + DeviceType, + MfaAuthenticateChallenge, + MfaChallengeResponse, +} from 'teleport/services/mfa'; import { getHttpFileTransferHandlers } from './httpFileTransferHandlers'; -import useGetScpUrl from './useGetScpUrl'; export type FileTransferRequest = { sid: string; @@ -60,17 +67,64 @@ export const useFileTransfer = ( const [fileTransferRequests, setFileTransferRequests] = useState< FileTransferRequest[] >([]); - const { getScpUrl, attempt: getMfaResponseAttempt } = - useGetScpUrl(addMfaToScpUrls); const { clusterId, serverId, login } = currentDoc; + const [mfaChallenge, setMfaChallenge] = useState(); + const mfaResponsePromise = + useRef>(); + + const clearMfaChallenge = () => { + setMfaChallenge(null); + }; + + const [mfaAttempt, getMfaResponse, setMfaAttempt] = useAsync( + useCallback(async () => { + const challenge = await auth.getMfaChallenge({ + scope: MfaChallengeScope.USER_SESSION, + isMfaRequiredRequest: { + node: { + node_name: serverId, + login: login, + }, + }, + }); + if (!challenge) return; + + setMfaChallenge(challenge); + mfaResponsePromise.current = Promise.withResolvers(); + const mfaResponse = await mfaResponsePromise.current.promise; + // clearMfaChallenge(); + return mfaResponse; + }, [setMfaChallenge, clearMfaChallenge, mfaResponsePromise]) + ); + + const [submitMfaAttempt, submitMfa] = useAsync( + useCallback( + async (mfaType?: DeviceType) => { + try { + const resp = auth.getMfaChallengeResponse(mfaChallenge, mfaType); + mfaResponsePromise.current.resolve(resp); + } catch (err) { + mfaResponsePromise.current.reject(err); + throw err; + } + }, + [mfaChallenge, mfaResponsePromise] + ) + ); + const download = useCallback( async ( location: string, abortController: AbortController, moderatedSessionParams?: ModeratedSessionParams ) => { - const url = await getScpUrl({ + let mfaResponse: MfaChallengeResponse; + if (addMfaToScpUrls) { + [mfaResponse] = await getMfaResponse(); + } + + const url = cfg.getScpUrl({ location, clusterId, serverId, @@ -78,7 +132,9 @@ export const useFileTransfer = ( filename: location, moderatedSessionId: moderatedSessionParams?.moderatedSessionId, fileTransferRequestId: moderatedSessionParams?.fileRequestId, + mfaResponse, }); + if (!url) { // if we return nothing here, the file transfer will not be added to the // file transfer list. If we add it to the list, the file will continue to @@ -88,7 +144,7 @@ export const useFileTransfer = ( } return getHttpFileTransferHandlers().download(url, abortController); }, - [clusterId, login, serverId, getScpUrl] + [clusterId, login, serverId, addMfaToScpUrls] ); const upload = useCallback( @@ -98,7 +154,12 @@ export const useFileTransfer = ( abortController: AbortController, moderatedSessionParams?: ModeratedSessionParams ) => { - const url = await getScpUrl({ + let mfaResponse: MfaChallengeResponse; + if (addMfaToScpUrls) { + [mfaResponse] = await getMfaResponse(); + } + + const url = cfg.getScpUrl({ location, clusterId, serverId, @@ -106,6 +167,7 @@ export const useFileTransfer = ( filename: file.name, moderatedSessionId: moderatedSessionParams?.moderatedSessionId, fileTransferRequestId: moderatedSessionParams?.fileRequestId, + mfaResponse, }); if (!url) { // if we return nothing here, the file transfer will not be added to the @@ -116,7 +178,7 @@ export const useFileTransfer = ( } return getHttpFileTransferHandlers().upload(url, file, abortController); }, - [clusterId, serverId, login, getScpUrl] + [clusterId, serverId, login, addMfaToScpUrls] ); /* @@ -255,8 +317,12 @@ export const useFileTransfer = ( } return { + mfaAttempt, + mfaChallenge, + clearMfaChallenge, + submitMfa, + submitMfaAttempt, fileTransferRequests, - getMfaResponseAttempt, getUploader, getDownloader, }; diff --git a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts deleted file mode 100644 index ac12807524d9c..0000000000000 --- a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts +++ /dev/null @@ -1,63 +0,0 @@ -/** - * Teleport - * Copyright (C) 2023 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 { useCallback } from 'react'; -import useAttempt from 'shared/hooks/useAttemptNext'; - -import cfg, { UrlScpParams } from 'teleport/config'; -import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; - -export default function useGetScpUrl(addMfaToScpUrls: boolean) { - const { setAttempt, attempt, handleError } = useAttempt(''); - - const getScpUrl = useCallback( - async (params: UrlScpParams) => { - setAttempt({ - status: 'processing', - statusText: '', - }); - if (!addMfaToScpUrls) { - return cfg.getScpUrl(params); - } - try { - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.USER_SESSION, - }); - - const mfaResponse = await auth.getMfaChallengeResponse(challenge); - - setAttempt({ - status: 'success', - statusText: '', - }); - return cfg.getScpUrl({ - mfaResponse, - ...params, - }); - } catch (error) { - handleError(error); - } - }, - [addMfaToScpUrls, handleError, setAttempt] - ); - - return { - getScpUrl, - attempt, - }; -} diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx index f867c976d5f8e..b67a5a7f5788b 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx @@ -25,11 +25,17 @@ import { ButtonIcon, ButtonSecondary, Flex, H2, Text } from 'design'; import { guessProviderType } from 'shared/components/ButtonSso'; import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso'; -import { MfaState } from 'teleport/lib/useMfa'; +import { Attempt } from 'shared/hooks/useAsync'; +import { DeviceType, MfaAuthenticateChallenge } from 'teleport/services/mfa'; -export default function AuthnDialog({ mfa, onCancel }: Props) { - let hasMultipleOptions = - mfa.mfaChallenge.ssoChallenge && mfa.mfaChallenge.webauthnPublicKey; +export default function AuthnDialog({ + mfaChallenge, + submitMfa, + submitAttempt, + onCancel, +}: Props) { + const hasMultipleOptions = + mfaChallenge.ssoChallenge && mfaChallenge.webauthnPublicKey; return ( ({ width: '400px' })} open={true}> @@ -40,9 +46,9 @@ export default function AuthnDialog({ mfa, onCancel }: Props) { - {mfa.submitAttempt.status === 'error' && ( + {submitAttempt.status === 'error' && ( - {mfa.submitAttempt.statusText} + {submitAttempt.statusText} )} @@ -52,28 +58,28 @@ export default function AuthnDialog({ mfa, onCancel }: Props) { - {mfa.mfaChallenge.ssoChallenge && ( + {mfaChallenge.ssoChallenge && ( mfa.submitMfa('sso')} + onClick={() => submitMfa('sso')} gap={2} block > - {mfa.mfaChallenge.ssoChallenge.device.displayName || - mfa.mfaChallenge.ssoChallenge.device.connectorId} + {mfaChallenge.ssoChallenge.device.displayName || + mfaChallenge.ssoChallenge.device.connectorId} )} - {mfa.mfaChallenge.webauthnPublicKey && ( + {mfaChallenge.webauthnPublicKey && ( mfa.submitMfa('webauthn')} + onClick={() => submitMfa('webauthn')} gap={2} block > @@ -87,6 +93,8 @@ export default function AuthnDialog({ mfa, onCancel }: Props) { } export type Props = { - mfa: MfaState; + mfaChallenge: MfaAuthenticateChallenge; + submitMfa: (mfaType?: DeviceType) => Promise; + submitAttempt: Attempt; onCancel: () => void; }; diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index 4b62b1968672d..71e65f219f18c 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -37,7 +37,8 @@ export function useMfa(emitterSender: EventEmitterMfaSender): MfaState { return auth .getMfaChallengeResponse(mfaChallenge, mfaType) - .then(res => emitterSender.sendChallengeResponse(res)); + .then(res => emitterSender.sendChallengeResponse(res)) + .then(() => setMfaChallenge(null)); }, [mfaChallenge] )