Skip to content

Commit

Permalink
Refactor cli mfa method preferences; remove mfa method preference fro…
Browse files Browse the repository at this point in the history 
…m Teleport Connect to enabled OTP.
  • Loading branch information
@Joerger
Joerger committed Oct 23, 2024
1 parent b993005 commit 4b5065b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 64 deletions.
78 changes: 67 additions & 11 deletions lib/client/mfa/cli.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"io"
"runtime"
"strings"
"sync"

"github.com/gravitational/trace"
Expand All @@ -35,6 +36,15 @@ import (
"github.com/gravitational/teleport/lib/auth/webauthnwin"
)

// CLIMFAType is the CLI display name for an MFA type.
type CLIMFAType string

const (
CLIMFATypeOTP = "OTP"
CLIMFATypeWebauthn = "WEBAUTHN"
CLIMFATypeSSO = "SSO"
)

// CLIPrompt is the default CLI mfa prompt implementation.
type CLIPrompt struct {
cfg PromptConfig
Expand Down Expand Up @@ -62,19 +72,65 @@ 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
promptWebauthn := chal.WebauthnChallenge != nil && c.cfg.WebauthnSupported
promptSSO := false // TODO(Joerger): check for SSO challenge once added in separate PR.

// No prompt to run, no-op.
if !runOpts.PromptTOTP && !runOpts.PromptWebauthn {
if !promptOTP && !promptWebauthn && !promptSSO {
return &proto.MFAAuthenticateResponse{}, nil
}

var availableMethods []string
if promptWebauthn {
availableMethods = append(availableMethods, CLIMFATypeWebauthn)
}
if promptSSO {
availableMethods = append(availableMethods, CLIMFATypeSSO)
}
if promptOTP {
availableMethods = append(availableMethods, CLIMFATypeOTP)
}

// Use stronger auth methods if hijack is not allowed.
if !c.cfg.AllowStdinHijack && (promptWebauthn || promptSSO) {
promptOTP = false
}

// Prefer Webauthn > SSO > OTP, or whatever method is requested or required by the client.
var chosenMethod string
switch {
case promptWebauthn && c.cfg.AuthenticatorAttachment != wancli.AttachmentAuto:
// Prefer Webauthn if a specific webauthn attachment was requested.
chosenMethod = CLIMFATypeWebauthn
promptSSO, promptOTP = false, false
case c.cfg.PreferSSO && promptSSO:
chosenMethod = CLIMFATypeSSO
promptWebauthn, promptOTP = false, false
case c.cfg.PreferOTP && promptOTP:
chosenMethod = CLIMFATypeOTP
promptWebauthn, promptSSO = false, false
case promptWebauthn:
// prefer webauthn over sso, but allow dual prompt with totp.
chosenMethod = CLIMFATypeWebauthn
promptSSO = false
if promptOTP {
chosenMethod = fmt.Sprintf("%v and %v", CLIMFATypeWebauthn, CLIMFATypeOTP)
}
case promptSSO:
// prefer sso over otp
chosenMethod = CLIMFATypeSSO
promptOTP = false
case promptOTP:
chosenMethod = CLIMFATypeOTP
}

fmt.Fprintf(c.writer, "Available MFA methods [%v]. Continuing with %v.\n", strings.Join(availableMethods, ", "), chosenMethod)
fmt.Fprintln(c.writer, "If you wish to perform MFA with another method, specify with flag --mfa-mode=<sso,otp>.")

// 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
dualPrompt := promptOTP && promptWebauthn

// Print the prompt message directly here in case of dualPrompt.
// This avoids problems with a goroutine failing before any message is
Expand All @@ -90,9 +146,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() {
Expand All @@ -109,13 +165,13 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
}()

quiet := c.cfg.Quiet || dualPrompt
resp, err := c.promptTOTP(otpCtx, quiet)
resp, err := c.promptOTP(otpCtx, quiet)
respC <- MFAGoroutineResponse{Resp: resp, Err: trace.Wrap(err, "TOTP authentication failed")}
}()
}

// Fire Webauthn goroutine.
if runOpts.PromptWebauthn {
if promptWebauthn {
wg.Add(1)
go func() {
defer func() {
Expand All @@ -139,7 +195,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
return HandleMFAPromptGoroutines(ctx, spawnGoroutines)
}

func (c *CLIPrompt) promptTOTP(ctx context.Context, quiet bool) (*proto.MFAAuthenticateResponse, error) {
func (c *CLIPrompt) promptOTP(ctx context.Context, quiet bool) (*proto.MFAAuthenticateResponse, error) {
var msg string
if !quiet {
msg = fmt.Sprintf("Enter an OTP code from a %sdevice", c.promptDevicePrefix())
Expand Down
38 changes: 3 additions & 35 deletions lib/client/mfa/prompt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand All @@ -84,41 +87,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
Expand Down
18 changes: 8 additions & 10 deletions lib/teleterm/daemon/mfaprompt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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)
Expand Down
8 changes: 0 additions & 8 deletions web/packages/teleterm/src/ui/ModalsHost/modals/ReAuthenticate/ReAuthenticate.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 4b5065b

Please sign in to comment.