Skip to content

Commit

Permalink
Make cli config a field rather than embedded; Add NewCLIPromptV2.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Oct 24, 2024
1 parent 916e670 commit a85d056
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 71 deletions.
16 changes: 7 additions & 9 deletions lib/client/mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,13 @@ type WebauthnLoginFunc = libmfa.WebauthnLoginFunc
func (tc *TeleportClient) NewMFAPrompt(opts ...mfa.PromptOpt) mfa.Prompt {
cfg := tc.newPromptConfig(opts...)

var prompt mfa.Prompt = &libmfa.CLIPrompt{
CLIPromptConfig: libmfa.CLIPromptConfig{
PromptConfig: *cfg,
Writer: tc.Stderr,
PreferOTP: tc.PreferOTP,
AllowStdinHijack: tc.AllowStdinHijack,
StdinFunc: tc.StdinFunc,
},
}
var prompt mfa.Prompt = libmfa.NewCLIPromptV2(&libmfa.CLIPromptConfig{
PromptConfig: *cfg,
Writer: tc.Stderr,
PreferOTP: tc.PreferOTP,
AllowStdinHijack: tc.AllowStdinHijack,
StdinFunc: tc.StdinFunc,
})

if tc.MFAPromptConstructor != nil {
prompt = tc.MFAPromptConstructor(cfg)
Expand Down
77 changes: 40 additions & 37 deletions lib/client/mfa/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@ import (
"github.com/gravitational/teleport/lib/auth/webauthnwin"
)

const (
// cliMFATypeOTP is the CLI display name for OTP.
cliMFATypeOTP = "OTP"
// cliMFATypeWebauthn is the CLI display name for Webauthn.
cliMFATypeWebauthn = "WEBAUTHN"
)

// CLIPromptConfig contains CLI prompt config options.
type CLIPromptConfig struct {
PromptConfig
Expand All @@ -65,38 +58,53 @@ type CLIPromptConfig struct {

// CLIPrompt is the default CLI mfa prompt implementation.
type CLIPrompt struct {
CLIPromptConfig
cfg CLIPromptConfig
}

// NewCLIPrompt returns a new CLI mfa prompt with the config and writer.
// TODO(Joerger): Delete once /e is no longer dependent on it.
func NewCLIPrompt(cfg *PromptConfig, writer io.Writer) *CLIPrompt {
// If no config is provided, use defaults (zero value).
if cfg == nil {
cfg = new(PromptConfig)
}
return NewCLIPromptV2(&CLIPromptConfig{
PromptConfig: *cfg,
Writer: writer,
})
}

// NewCLIPromptV2 returns a new CLI mfa prompt with the given config.
// TODO(Joerger): this is V2 because /e depends on a different function
// signature for NewCLIPrompt, so this requires a couple follow up PRs to fix.
func NewCLIPromptV2(cfg *CLIPromptConfig) *CLIPrompt {
// If no config is provided, use defaults (zero value).
if cfg == nil {
cfg = new(CLIPromptConfig)
}
return &CLIPrompt{
CLIPromptConfig: CLIPromptConfig{
PromptConfig: *cfg,
Writer: writer,
},
cfg: *cfg,
}
}

func (c *CLIPrompt) stdin() prompt.StdinReader {
if c.StdinFunc == nil {
if c.cfg.StdinFunc == nil {
return prompt.Stdin()
}
return c.StdinFunc()
return c.cfg.StdinFunc()
}

func (c *CLIPrompt) writer() io.Writer {
if c.Writer == nil {
if c.cfg.Writer == nil {
return os.Stderr
}
return c.Writer
return c.cfg.Writer
}

// Run prompts the user to complete an MFA authentication challenge.
func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
if c.PromptReason != "" {
fmt.Fprintln(c.writer(), c.PromptReason)
if c.cfg.PromptReason != "" {
fmt.Fprintln(c.writer(), c.cfg.PromptReason)
}

promptOTP := chal.TOTP != nil
Expand All @@ -108,7 +116,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
}

// Check off unsupported methods.
if promptWebauthn && !c.WebauthnSupported {
if promptWebauthn && !c.cfg.WebauthnSupported {
promptWebauthn = false
slog.DebugContext(ctx, "hardware device MFA not supported by your platform")
if !promptOTP {
Expand All @@ -117,24 +125,19 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
}

// Prefer whatever method is requested by the client.
switch {
case c.PreferOTP && promptOTP:
if c.cfg.PreferOTP && promptOTP {
promptWebauthn = false
}

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

// If we have multiple viable options, prefer Webauthn > OTP.
switch {
case promptWebauthn:
// If a specific webauthn attachment was requested, skip OTP.
// Otherwise, allow dual prompt with OTP.
if c.AuthenticatorAttachment != wancli.AttachmentAuto {
promptOTP = false
}
// If a specific webauthn attachment was requested, skip OTP.
// Otherwise, allow dual prompt with OTP.
if promptWebauthn && c.cfg.AuthenticatorAttachment != wancli.AttachmentAuto {
promptOTP = false
}

switch {
Expand All @@ -145,7 +148,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng
resp, err := c.promptWebauthn(ctx, chal, c.getWebauthnPrompt(ctx))
return resp, trace.Wrap(err)
case promptOTP:
resp, err := c.promptOTP(ctx, c.Quiet)
resp, err := c.promptOTP(ctx, c.cfg.Quiet)
return resp, trace.Wrap(err)
default:
// We shouldn't reach this case as we would have hit the no-op case above.
Expand Down Expand Up @@ -173,20 +176,20 @@ func (c *CLIPrompt) promptOTP(ctx context.Context, quiet bool) (*proto.MFAAuthen

func (c *CLIPrompt) getWebauthnPrompt(ctx context.Context) *wancli.DefaultPrompt {
writer := c.writer()
if c.Quiet {
if c.cfg.Quiet {
writer = io.Discard
}

prompt := wancli.NewDefaultPrompt(ctx, writer)
prompt.StdinFunc = c.StdinFunc
prompt.StdinFunc = c.cfg.StdinFunc
prompt.SecondTouchMessage = fmt.Sprintf("Tap your %ssecurity key to complete login", c.promptDevicePrefix())
prompt.FirstTouchMessage = fmt.Sprintf("Tap any %ssecurity key", c.promptDevicePrefix())
return prompt
}

func (c *CLIPrompt) promptWebauthn(ctx context.Context, chal *proto.MFAAuthenticateChallenge, prompt wancli.LoginPrompt) (*proto.MFAAuthenticateResponse, error) {
opts := &wancli.LoginOpts{AuthenticatorAttachment: c.AuthenticatorAttachment}
resp, _, err := c.WebauthnLoginFunc(ctx, c.GetWebauthnOrigin(), wantypes.CredentialAssertionFromProto(chal.WebauthnChallenge), prompt, opts)
opts := &wancli.LoginOpts{AuthenticatorAttachment: c.cfg.AuthenticatorAttachment}
resp, _, err := c.cfg.WebauthnLoginFunc(ctx, c.cfg.GetWebauthnOrigin(), wantypes.CredentialAssertionFromProto(chal.WebauthnChallenge), prompt, opts)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -195,8 +198,8 @@ func (c *CLIPrompt) promptWebauthn(ctx context.Context, chal *proto.MFAAuthentic
}

func (c *CLIPrompt) promptDevicePrefix() string {
if c.DeviceType != "" {
return fmt.Sprintf("*%s* ", c.DeviceType)
if c.cfg.DeviceType != "" {
return fmt.Sprintf("*%s* ", c.cfg.DeviceType)
}
return ""
}
Expand Down
12 changes: 5 additions & 7 deletions lib/client/mfa/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,11 @@ Enter your security key PIN:
buffer := make([]byte, 0, 100)
out := bytes.NewBuffer(buffer)

prompt := &mfa.CLIPrompt{
CLIPromptConfig: mfa.CLIPromptConfig{
PromptConfig: *cfg,
Writer: out,
AllowStdinHijack: true,
},
}
prompt := mfa.NewCLIPromptV2(&mfa.CLIPromptConfig{
PromptConfig: *cfg,
Writer: out,
AllowStdinHijack: true,
})
resp, err := prompt.Run(ctx, tc.challenge)

if tc.expectErr != nil {
Expand Down
14 changes: 6 additions & 8 deletions lib/client/mfa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestPromptMFAChallenge_usingNonRegisteredDevice(t *testing.T) {
tests := []struct {
name string
challenge *proto.MFAAuthenticateChallenge
customizePrompt func(p *mfa.CLIPrompt)
customizePrompt func(p *mfa.CLIPromptConfig)
}{
{
name: "webauthn only",
Expand All @@ -81,7 +81,7 @@ func TestPromptMFAChallenge_usingNonRegisteredDevice(t *testing.T) {
{
name: "webauthn and OTP",
challenge: challengeWebauthnOTP,
customizePrompt: func(p *mfa.CLIPrompt) {
customizePrompt: func(p *mfa.CLIPromptConfig) {
p.AllowStdinHijack = true // required for OTP+WebAuthn prompt.
},
},
Expand All @@ -108,17 +108,15 @@ func TestPromptMFAChallenge_usingNonRegisteredDevice(t *testing.T) {
return nil, "", wancli.ErrUsingNonRegisteredDevice
}

prompt := &mfa.CLIPrompt{
CLIPromptConfig: mfa.CLIPromptConfig{
PromptConfig: *promptConfig,
},
cliConfig := &mfa.CLIPromptConfig{
PromptConfig: *promptConfig,
}

if test.customizePrompt != nil {
test.customizePrompt(prompt)
test.customizePrompt(cliConfig)
}

_, err := prompt.Run(ctx, test.challenge)
_, err := mfa.NewCLIPromptV2(cliConfig).Run(ctx, test.challenge)
if !errors.Is(err, wancli.ErrUsingNonRegisteredDevice) {
t.Errorf("PromptMFAChallenge returned err=%q, want %q", err, wancli.ErrUsingNonRegisteredDevice)
}
Expand Down
8 changes: 3 additions & 5 deletions tool/tctl/common/admin_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1029,11 +1029,9 @@ func newAdminActionTestSuite(t *testing.T) *adminActionTestSuite {
promptCfg := libmfa.NewPromptConfig(proxyPublicAddr.String(), opts...)
promptCfg.WebauthnLoginFunc = mockWebauthnLogin
promptCfg.WebauthnSupported = true
return &libmfa.CLIPrompt{
CLIPromptConfig: libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
},
}
return libmfa.NewCLIPromptV2(&libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
})
}

// Login as the admin user.
Expand Down
8 changes: 3 additions & 5 deletions tool/tctl/common/tctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,9 @@ func TryRun(commands []CLICommand, args []string) error {
proxyAddr := resp.ProxyPublicAddr
client.SetMFAPromptConstructor(func(opts ...mfa.PromptOpt) mfa.Prompt {
promptCfg := libmfa.NewPromptConfig(proxyAddr, opts...)
return &libmfa.CLIPrompt{
CLIPromptConfig: libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
},
}
return libmfa.NewCLIPromptV2(&libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
})
})

// execute whatever is selected:
Expand Down

0 comments on commit a85d056

Please # to comment.