Improve per-session MFA in a desktop session

lib/web/desktop.go

@@ -385,6 +385,9 @@ func (h *Handler) performSessionMFACeremony(
 			return mfa.PromptFunc(func(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
 				codec := tdpMFACodec{}
 
+				if chal.WebauthnChallenge == nil {
+					return nil, trace.AccessDenied("Desktop access requires WebAuthn MFA, please register a WebAuthn device to connect")
+				}
 				// Send the challenge over the socket.
 				msg, err := codec.Encode(
 					&client.MFAAuthenticateChallenge{

web/packages/teleport/src/DesktopSession/DesktopSession.tsx

@@ -26,13 +26,14 @@ import Dialog, {
   DialogHeader,
   DialogTitle,
 } from 'design/Dialog';
+import { Attempt as AsyncAttempt } from 'shared/hooks/useAsync';
 import { Attempt } from 'shared/hooks/useAttemptNext';
 
 import AuthnDialog from 'teleport/components/AuthnDialog';
 import TdpClientCanvas from 'teleport/components/TdpClientCanvas';
 import { TdpClientCanvasRef } from 'teleport/components/TdpClientCanvas/TdpClientCanvas';
 import { useListener } from 'teleport/lib/tdp/client';
-import type { MfaState } from 'teleport/lib/useMfa';
+import { MfaState, shouldShowMfaPrompt } from 'teleport/lib/useMfa';
 
 import TopBar from './TopBar';
 import useDesktopSession, {
@@ -257,7 +258,7 @@ export function DesktopSession(props: State) {
       {screenState.screen === 'anotherSessionActive' && (
         <AnotherSessionActiveDialog {...props} />
       )}
-      {screenState.screen === 'mfa' && <MfaDialog mfa={mfa} />}
+      {screenState.screen === 'mfa' && <AuthnDialog mfaState={mfa} />}
       {screenState.screen === 'alert dialog' && (
         <AlertDialog screenState={screenState} />
       )}
@@ -282,27 +283,20 @@ export function DesktopSession(props: State) {
   );
 }
 
-const MfaDialog = ({ mfa }: { mfa: MfaState }) => {
-  return (
-    <AuthnDialog
-      mfaState={mfa}
-      replaceErrorText={
-        'This session requires multi factor authentication to continue. Please hit try again and follow the prompts given by your browser to complete authentication.'
-      }
-    />
-  );
-};
-
 const AlertDialog = ({ screenState }: { screenState: ScreenState }) => (
   <Dialog dialogCss={() => ({ width: '484px' })} open={true}>
     <DialogHeader style={{ flexDirection: 'column' }}>
       <DialogTitle>Disconnected</DialogTitle>
     </DialogHeader>
     <DialogContent>
       <>
-        <Info
-          children={<>{screenState.alertMessage || invalidStateMessage}</>}
-        />
+        {typeof screenState.alertMessage === 'object' ? (
+          <Info details={screenState.alertMessage.message}>
+            {screenState.alertMessage.title}
+          </Info>
+        ) : (
+          <Info>{screenState.alertMessage}</Info>
+        )}
         Refresh the page to reconnect.
       </>
     </DialogContent>
@@ -376,7 +370,7 @@ const nextScreenState = (
   tdpConnection: Attempt,
   wsConnection: WebsocketAttempt,
   showAnotherSessionActiveDialog: boolean,
-  webauthn: MfaState
+  mfa: MfaState
 ): ScreenState => {
   // We always want to show the user the first alert that caused the session to fail/end,
   // so if we're already showing an alert, don't change the screen.
@@ -393,10 +387,11 @@ const nextScreenState = (
 
   // Otherwise, calculate a new screen state.
   const showAnotherSessionActive = showAnotherSessionActiveDialog;
-  const showMfa = webauthn.challenge;
+  const showMfa = shouldShowMfaPrompt(mfa);
   const showAlert =
     fetchAttempt.status === 'failed' || // Fetch attempt failed
     tdpConnection.status === 'failed' || // TDP connection closed by the remote side.
+    mfa.attempt.status === 'error' || // MFA was canceled
     wsConnection.status === 'closed'; // Websocket closed, means unexpected close.
 
   const atLeastOneAttemptProcessing =
@@ -431,6 +426,7 @@ const nextScreenState = (
         tdpConnection,
         wsConnection,
         showAnotherSessionActiveDialog,
+        mfa.attempt,
         prevState
       ),
       canvasState: { shouldConnect: false, shouldDisplay: false },
@@ -461,9 +457,17 @@ const calculateAlertMessage = (
   tdpConnection: Attempt,
   wsConnection: WebsocketAttempt,
   showAnotherSessionActiveDialog: boolean,
+  mfaAttempt: AsyncAttempt<unknown>,
   prevState: ScreenState
-): string => {
+) => {
   let message = '';
+  // Errors, except for dialog cancellations, are handled within the MFA dialog.
+  if (mfaAttempt.status === 'error') {
+    return {
+      title: 'This session requires multi factor authentication',
+      message: mfaAttempt.statusText,
+    };
+  }
   if (fetchAttempt.status === 'failed') {
     message = fetchAttempt.statusText || 'fetch attempt failed';
   } else if (tdpConnection.status === 'failed') {
@@ -494,7 +498,7 @@ type ScreenState = {
     | 'processing'
     | 'canvas';
 
-  alertMessage?: string;
+  alertMessage?: string | { title: string; message: string };
   canvasState: {
     shouldConnect: boolean;
     shouldDisplay: boolean;

web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx

@@ -23,26 +23,21 @@ import { Cross, FingerprintSimple } from 'design/Icon';
 import { guessProviderType } from 'shared/components/ButtonSso';
 import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso';
 
-import { MfaCanceledError, MfaState } from 'teleport/lib/useMfa';
+import { MfaState, shouldShowMfaPrompt } from 'teleport/lib/useMfa';
 import { MFA_OPTION_TOTP } from 'teleport/services/mfa';
 
 export type Props = {
   mfaState: MfaState;
-  replaceErrorText?: string;
   // onClose is an optional function to perform additional operations
   // upon closing the dialog. e.g. close a shell session
   onClose?: () => void;
 };
 
 export default function AuthnDialog({
   mfaState: { options, challenge, submit, attempt, cancelAttempt },
-  replaceErrorText,
   onClose = () => {},
 }: Props) {
-  const showError =
-    attempt.status === 'error' && !(attempt.error instanceof MfaCanceledError);
-
-  if (!challenge && !showError) return;
+  if (!shouldShowMfaPrompt({ challenge, attempt })) return;
 
   // TODO(Joerger): TOTP should be pretty easy to support here with a small button -> form flow.
   const onlyTotpAvailable =
@@ -72,7 +67,7 @@ export default function AuthnDialog({
         )}
         {attempt.status === 'error' && (
           <Danger data-testid="danger-alert" mt={2} width="100%">
-            {replaceErrorText || attempt.statusText}
+            {attempt.statusText}
           </Danger>
         )}
         <Text color="text.slightlyMuted">

web/packages/teleport/src/lib/useMfa.test.tsx

@@ -118,6 +118,15 @@ describe('useMfa', () => {
     await waitFor(() => expect(mfa.current.mfaRequired).toEqual(false));
   });
 
+  test('returns empty challenge response when mfa challenge and request are absent', async () => {
+    const { result: mfa } = renderHook(() => useMfa());
+
+    const resp = await act(() => mfa.current.getChallengeResponse());
+
+    expect(resp).toBeUndefined();
+    expect(mfa.current.mfaRequired).toEqual(false);
+  });
+
   test('adaptable mfa requirement', async () => {
     jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(mockChallenge);
     jest
@@ -175,7 +184,7 @@ describe('useMfa', () => {
       throw err;
     });
 
-    const { result: mfa } = renderHook(() => useMfa({}));
+    const { result: mfa } = renderHook(() => useMfa({ req: mockChallengeReq }));
 
     await act(async () => {
       await expect(mfa.current.getChallengeResponse()).rejects.toThrow(err);

web/packages/teleport/src/lib/useMfa.ts

@@ -84,23 +84,20 @@ export function useMfa(props?: MfaProps): MfaState {
         }
 
         // Caller didn't pass a challenge and the mfa required is true or unknown.
-        if (!challenge) {
-          let req = props.req;
-
+        if (!challenge && props?.req) {
           // We already know MFA is required, skip the extra check.
-          if (mfaRequired === true) req.isMfaRequiredRequest = null;
-
+          if (mfaRequired === true) props.req.isMfaRequiredRequest = null;
           challenge = await auth.getMfaChallenge(props.req);
+        }
 
-          // An empty challenge means either mfa is not required, or the user has no mfa devices.
-          if (!challenge) {
-            setMfaRequired(false);
-            return;
-          }
-
-          setMfaRequired(true);
+        // An empty challenge means either mfa is not required, or the user has no mfa devices.
+        if (!challenge) {
+          setMfaRequired(false);
+          return;
         }
 
+        setMfaRequired(true);
+
         // Prepare a new promise to collect the mfa response retrieved
         // through the submit function.
         let resolve: (value: MfaChallengeResponse) => void;
@@ -218,6 +215,17 @@ export type MfaState = {
   cancelAttempt: () => void;
 };
 
+/** Indicates if an MFA dialog should be visible. */
+export function shouldShowMfaPrompt(
+  mfa: Pick<MfaState, 'challenge' | 'attempt'>
+): boolean {
+  return (
+    !!mfa.challenge ||
+    (mfa.attempt.status === 'error' &&
+      !(mfa.attempt.error instanceof MfaCanceledError))
+  );
+}
+
 // used for testing
 export function makeDefaultMfaState(): MfaState {
   return {