Allow Teleport Connect users to choose either WebAuthn or OTP in MFA …

…prompts. (#47875)

lib/client/mfa/prompt.go

@@ -51,17 +51,8 @@ type PromptConfig struct {
 	ProxyAddress string
 	// WebauthnLoginFunc performs client-side Webauthn login.
 	WebauthnLoginFunc WebauthnLoginFunc
-	// AllowStdinHijack allows stdin hijack during MFA prompts.
-	// Stdin hijack provides a better login UX, but it can be difficult to reason
-	// about and is often a source of bugs.
-	// Do not set this options unless you deeply understand what you are doing.
-	// If false then only the strongest auth method is prompted.
-	AllowStdinHijack bool
 	// AuthenticatorAttachment specifies the desired authenticator attachment.
 	AuthenticatorAttachment wancli.AuthenticatorAttachment
-	// PreferOTP favors OTP challenges, if applicable.
-	// Takes precedence over AuthenticatorAttachment settings.
-	PreferOTP bool
 	// WebauthnSupported indicates whether Webauthn is supported.
 	WebauthnSupported bool
 }
@@ -81,41 +72,6 @@ func NewPromptConfig(proxyAddr string, opts ...mfa.PromptOpt) *PromptConfig {
 	return cfg
 }
 
-// RunOpts are mfa prompt run options.
-type RunOpts struct {
-	PromptTOTP     bool
-	PromptWebauthn bool
-}
-
-// GetRunOptions gets mfa prompt run options by cross referencing the mfa challenge with prompt configuration.
-func (c PromptConfig) GetRunOptions(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (RunOpts, error) {
-	promptTOTP := chal.TOTP != nil
-	promptWebauthn := chal.WebauthnChallenge != nil
-
-	// Does the current platform support hardware MFA? Adjust accordingly.
-	switch {
-	case !promptTOTP && promptWebauthn && !c.WebauthnSupported:
-		return RunOpts{}, trace.BadParameter("hardware device MFA not supported by your platform, please register an OTP device")
-	case !c.WebauthnSupported:
-		// Do not prompt for hardware devices, it won't work.
-		promptWebauthn = false
-	}
-
-	// Tweak enabled/disabled methods according to opts.
-	switch {
-	case promptTOTP && c.PreferOTP:
-		promptWebauthn = false
-	case promptWebauthn && c.AuthenticatorAttachment != wancli.AttachmentAuto:
-		// Prefer Webauthn if an specific attachment was requested.
-		promptTOTP = false
-	case promptWebauthn && !c.AllowStdinHijack:
-		// Use strongest auth if hijack is not allowed.
-		promptTOTP = false
-	}
-
-	return RunOpts{promptTOTP, promptWebauthn}, nil
-}
-
 func (c PromptConfig) GetWebauthnOrigin() string {
 	if !strings.HasPrefix(c.ProxyAddress, "https://") {
 		return "https://" + c.ProxyAddress

lib/teleterm/clusters/storage.go

@@ -285,17 +285,6 @@ func (s *Storage) makeDefaultClientConfig(rootClusterURI uri.ResourceURI) *clien
 	cfg.InsecureSkipVerify = s.InsecureSkipVerify
 	cfg.AddKeysToAgent = s.AddKeysToAgent
 	cfg.WebauthnLogin = s.WebauthnLogin
-	// Set AllowStdinHijack to true to enable daemon.mfaPrompt to ask for both TOTP and Webauthn at
-	// the same time if available.
-	//
-	// tsh sets AllowStdinHijack to true only during tsh login to avoid input swallowing bugs where
-	// calling a command would prompt for MFA and then expect some further data through stdin. tsh
-	// login does not ask for any further input after the MFA prompt.
-	//
-	// Since tsh daemon ran by Connect never expects data over stdin, it can always set this flag to
-	// true.
-	cfg.AllowStdinHijack = true
-
 	cfg.CustomHardwareKeyPrompt = s.HardwareKeyPromptConstructor(rootClusterURI)
 	cfg.DTAuthnRunCeremony = dtauthn.NewCeremony().Run
 	cfg.DTAutoEnroll = dtenroll.AutoEnroll

lib/teleterm/daemon/mfaprompt.go

@@ -73,13 +73,11 @@ func (s *Service) promptAppMFA(ctx context.Context, in *api.PromptMFARequest) (*
 
 // Run prompts the user to complete an MFA authentication challenge.
 func (p *mfaPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
-	runOpts, err := p.cfg.GetRunOptions(ctx, chal)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
+	promptOTP := chal.TOTP != nil
+	promptWebauthn := chal.WebauthnChallenge != nil && p.cfg.WebauthnSupported
 
 	// No prompt to run, no-op.
-	if !runOpts.PromptTOTP && !runOpts.PromptWebauthn {
+	if !promptOTP && !promptWebauthn {
 		return &proto.MFAAuthenticateResponse{}, nil
 	}
 
@@ -92,7 +90,7 @@ func (p *mfaPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 		go func() {
 			defer wg.Done()
 
-			resp, err := p.promptMFA(ctx, runOpts)
+			resp, err := p.promptMFA(ctx, promptOTP, promptWebauthn)
 			respC <- libmfa.MFAGoroutineResponse{Resp: resp, Err: err}
 
 			// If the user closes the modal in the Electron app, we need to be able to cancel the other
@@ -103,7 +101,7 @@ func (p *mfaPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 		}()
 
 		// Fire Webauthn goroutine.
-		if runOpts.PromptWebauthn {
+		if promptWebauthn {
 			wg.Add(1)
 			go func() {
 				defer wg.Done()
@@ -128,12 +126,12 @@ func (p *mfaPrompt) promptWebauthn(ctx context.Context, chal *proto.MFAAuthentic
 	return resp, nil
 }
 
-func (p *mfaPrompt) promptMFA(ctx context.Context, runOpts libmfa.RunOpts) (*proto.MFAAuthenticateResponse, error) {
+func (p *mfaPrompt) promptMFA(ctx context.Context, promptOTP, promptWebauthn bool) (*proto.MFAAuthenticateResponse, error) {
 	resp, err := p.promptAppMFA(ctx, &api.PromptMFARequest{
 		ClusterUri: p.resourceURI.GetClusterURI().String(),
 		Reason:     p.cfg.PromptReason,
-		Totp:       runOpts.PromptTOTP,
-		Webauthn:   runOpts.PromptWebauthn,
+		Totp:       promptOTP,
+		Webauthn:   promptWebauthn,
 	})
 	if err != nil {
 		return nil, trail.FromGRPC(err)

web/packages/teleterm/src/ui/ModalsHost/modals/ReAuthenticate/ReAuthenticate.tsx

@@ -61,14 +61,6 @@ export const ReAuthenticate: FC<{
   const logger = useLogger('ReAuthenticate');
   const { promptMfaRequest: req } = props;
 
-  // TODO(ravicious): At the moment it doesn't seem like it's possible for both Webauthn and TOTP to
-  // be available at the same time (see lib/client/mfa.PromptConfig/GetRunOptions). Whenever both
-  // Webauthn and TOTP are supported, Webauthn is preferred. Well, unless AllowStdinHijack is
-  // specified, but lib/teleterm doesn't do this and AllowStdinHijack has a scary comment next to it
-  // telling you not to use it.
-  //
-  // Alas, the data structure certainly allows for this so the modal was designed with supporting
-  // such scenario in mind.
   const availableMfaTypes: MfaType[] = [];
   // Add Webauthn first to prioritize it if both Webauthn and TOTP are available.
   if (req.webauthn) {