Refactor mfa method preferences for SSO MFA.

gravitational · Oct 22, 2024 · c594570 · c594570
1 parent 7880566
commit c594570
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 54 deletions.
diff --git a/lib/client/mfa/cli.go b/lib/client/mfa/cli.go
@@ -62,23 +62,37 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 		fmt.Fprintln(c.writer, c.cfg.PromptReason)
 	}
 
-	runOpts, err := c.cfg.GetRunOptions(ctx, chal)
-	if err != nil {
-		return nil, trace.Wrap(err)
+	promptOTP := chal.TOTP != nil
+	promptSSO := chal.SSOChallenge != nil && c.cfg.SSOMFACeremony != nil
+	promptWebauthn := chal.WebauthnChallenge != nil && c.cfg.WebauthnSupported
+
+	// Prefer Webauthn > SSO > OTP, or whatever method is requested or required by the client.
+	switch {
+	case promptWebauthn && c.cfg.AuthenticatorAttachment != wancli.AttachmentAuto:
+		// Prefer Webauthn if a specific webauthn attachment was requested.
+		promptSSO, promptOTP = false, false
+	case c.cfg.PreferSSO && promptSSO:
+		promptWebauthn, promptOTP = false, false
+	case c.cfg.PreferOTP && promptOTP:
+		promptWebauthn, promptSSO = false, false
+	case promptWebauthn && promptSSO:
+		// prefer webauthn over sso
+		promptSSO = false
+	case promptSSO && promptOTP:
+		// prefer sso over otp
+		promptOTP = false
 	}
 
 	// No prompt to run, no-op.
-	if !runOpts.PromptTOTP && !runOpts.PromptWebauthn && !runOpts.PromptSSO {
+	if !promptOTP && !promptSSO && !promptWebauthn {
 		return &proto.MFAAuthenticateResponse{}, nil
 	}
 
-	// TODO: Rework prompt logic to display options and select one automatically. Login should still
-	// switch between OTP and WebAuthn, until we detect when webauthn key plugged in.
+	// WebAuthn+OTP is the only dual prompt supported and is only currently utilized in login.
+	dualPrompt := promptOTP && promptWebauthn
 
 	// Depending on the run opts, we may spawn a TOTP goroutine, webauth goroutine, or both.
 	spawnGoroutines := func(ctx context.Context, wg *sync.WaitGroup, respC chan<- MFAGoroutineResponse) {
-		dualPrompt := runOpts.PromptTOTP && runOpts.PromptWebauthn
-
 		// Print the prompt message directly here in case of dualPrompt.
 		// This avoids problems with a goroutine failing before any message is
 		// printed.
@@ -93,9 +107,9 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 			fmt.Fprintln(c.writer, message)
 		}
 
-		// Fire TOTP goroutine.
+		// Fire OTP goroutine.
 		var otpCancelAndWait func()
-		if runOpts.PromptTOTP {
+		if promptOTP {
 			otpCtx, otpCancel := context.WithCancel(ctx)
 			otpDone := make(chan struct{})
 			otpCancelAndWait = func() {
@@ -118,7 +132,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 		}
 
 		// Fire Webauthn goroutine.
-		if runOpts.PromptWebauthn {
+		if promptWebauthn {
 			wg.Add(1)
 			go func() {
 				defer func() {
@@ -139,7 +153,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
 		}
 
 		// Fire SSO goroutine.
-		if runOpts.PromptSSO {
+		if promptSSO {
 			wg.Add(1)
 			go func() {
 				defer wg.Done()

diff --git a/lib/client/mfa/prompt.go b/lib/client/mfa/prompt.go
@@ -62,6 +62,9 @@ type PromptConfig struct {
 	// PreferOTP favors OTP challenges, if applicable.
 	// Takes precedence over AuthenticatorAttachment settings.
 	PreferOTP bool
+	// PreferSSO favors SSO challenges, if applicable.
+	// Takes precedence over AuthenticatorAttachment settings.
+	PreferSSO bool
 	// WebauthnSupported indicates whether Webauthn is supported.
 	WebauthnSupported bool
 	// StdinFunc allows tests to override prompt.Stdin().
@@ -91,38 +94,6 @@ type RunOpts struct {
 	PromptSSO      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
-	promptSSO := chal.SSOChallenge != nil && c.SSOMFACeremony != nil
-
-	// TODO: rework preference logic for webauthn > SSO > OTP.
-
-	// 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, promptSSO}, nil
-}
-
 func (c PromptConfig) GetWebauthnOrigin() string {
 	if !strings.HasPrefix(c.ProxyAddress, "https://") {
 		return "https://" + c.ProxyAddress

diff --git a/lib/teleterm/daemon/mfaprompt.go b/lib/teleterm/daemon/mfaprompt.go
@@ -73,13 +73,12 @@ 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
+	promptSSO := chal.SSOChallenge != nil && p.cfg.SSOMFACeremony != nil
+	promptWebauthn := chal.WebauthnChallenge != nil && p.cfg.WebauthnSupported
 
 	// No prompt to run, no-op.
-	if !runOpts.PromptTOTP && !runOpts.PromptWebauthn {
+	if !promptOTP && !promptSSO && !promptWebauthn {
 		return &proto.MFAAuthenticateResponse{}, nil
 	}
 
@@ -92,7 +91,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 +102,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 +127,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)

diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go
@@ -117,6 +117,8 @@ const (
 	mfaModePlatform = "platform"
 	// mfaModeOTP utilizes only OTP devices.
 	mfaModeOTP = "otp"
+	// mfaModeSSO utilizes only SSO MFA devices.
+	mfaModeSSO = "sso"
 )
 
 const (