Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CLI-based config options from Teleport Connect MFA prompts #47875

Draft
wants to merge 1 commit into
base: joerger/cli-prompt-refactor
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 0 additions & 44 deletions lib/client/mfa/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
18 changes: 8 additions & 10 deletions lib/teleterm/daemon/mfaprompt.go
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
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
Loading