Skip to content

Commit

Permalink
Replace NewCLIPromptV2.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Oct 30, 2024
1 parent 6e5b580 commit f282b69
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 37 deletions.
2 changes: 1 addition & 1 deletion lib/client/mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type WebauthnLoginFunc = libmfa.WebauthnLoginFunc
func (tc *TeleportClient) NewMFAPrompt(opts ...mfa.PromptOpt) mfa.Prompt {
cfg := tc.newPromptConfig(opts...)

var prompt mfa.Prompt = libmfa.NewCLIPromptV2(&libmfa.CLIPromptConfig{
var prompt mfa.Prompt = libmfa.NewCLIPrompt(&libmfa.CLIPromptConfig{
PromptConfig: *cfg,
Writer: tc.Stderr,
PreferOTP: tc.PreferOTP,
Expand Down
25 changes: 8 additions & 17 deletions lib/client/mfa/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,23 +75,8 @@ type CLIPrompt struct {
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 {
// NewCLIPrompt returns a new CLI mfa prompt with the given config.
func NewCLIPrompt(cfg *CLIPromptConfig) *CLIPrompt {
// If no config is provided, use defaults (zero value).
if cfg == nil {
cfg = new(CLIPromptConfig)
Expand All @@ -101,6 +86,12 @@ func NewCLIPromptV2(cfg *CLIPromptConfig) *CLIPrompt {
}
}

// NewCLIPromptV2 returns a new CLI mfa prompt with the given config.
// TODO(Joerger): remove once /e is no longer dependent on this.
func NewCLIPromptV2(cfg *CLIPromptConfig) *CLIPrompt {
return NewCLIPrompt(cfg)
}

func (c *CLIPrompt) stdin() prompt.StdinReader {
if c.cfg.StdinFunc == nil {
return prompt.Stdin()
Expand Down
47 changes: 31 additions & 16 deletions lib/client/mfa/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ func TestCLIPrompt(t *testing.T) {
expectStdOut: "",
challenge: &proto.MFAAuthenticateChallenge{},
expectResp: &proto.MFAAuthenticateResponse{},
}, {
},
{
name: "OK webauthn",
expectStdOut: "Tap any security key\n",
challenge: &proto.MFAAuthenticateChallenge{
Expand All @@ -65,7 +66,8 @@ func TestCLIPrompt(t *testing.T) {
Webauthn: &webauthnpb.CredentialAssertionResponse{},
},
},
}, {
},
{
name: "OK otp",
expectStdOut: "Enter an OTP code from a device:\n",
stdin: "123456",
Expand All @@ -79,7 +81,8 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "OK sso",
expectStdOut: "", // sso stdout is handled internally in the SSO ceremony, which is mocked in this test.
challenge: &proto.MFAAuthenticateChallenge{
Expand All @@ -93,7 +96,8 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "OK prefer otp when specified",
expectStdOut: "Enter an OTP code from a device:\n",
stdin: "123456",
Expand All @@ -112,7 +116,8 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "OK prefer sso when specified",
expectStdOut: "",
challenge: &proto.MFAAuthenticateChallenge{
Expand All @@ -131,7 +136,8 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "OK prefer webauthn with authenticator attachment requested",
expectStdOut: "Tap any security key\n",
challenge: &proto.MFAAuthenticateChallenge{
Expand Down Expand Up @@ -163,7 +169,8 @@ func TestCLIPrompt(t *testing.T) {
Webauthn: &webauthnpb.CredentialAssertionResponse{},
},
},
}, {
},
{
name: "OK prefer webauthn+otp over sso",
expectStdOut: "" +
"Available MFA methods [WEBAUTHN, SSO, OTP]. Continuing with WEBAUTHN and OTP.\n" +
Expand All @@ -182,7 +189,8 @@ func TestCLIPrompt(t *testing.T) {
Webauthn: &webauthnpb.CredentialAssertionResponse{},
},
},
}, {
},
{
name: "OK prefer sso over otp",
expectStdOut: "" +
"Available MFA methods [SSO, OTP]. Continuing with SSO.\n" +
Expand All @@ -199,7 +207,8 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "OK prefer webauthn over otp when stdin hijack disallowed",
expectStdOut: "" +
"Available MFA methods [WEBAUTHN, OTP]. Continuing with WEBAUTHN.\n" +
Expand All @@ -214,7 +223,8 @@ func TestCLIPrompt(t *testing.T) {
Webauthn: &webauthnpb.CredentialAssertionResponse{},
},
},
}, {
},
{
name: "OK webauthn or otp with stdin hijack allowed, choose webauthn",
expectStdOut: "" +
"Available MFA methods [WEBAUTHN, SSO, OTP]. Continuing with WEBAUTHN and OTP.\n" +
Expand All @@ -233,7 +243,8 @@ func TestCLIPrompt(t *testing.T) {
Webauthn: &webauthnpb.CredentialAssertionResponse{},
},
},
}, {
},
{
name: "OK webauthn or otp with stdin hijack allowed, choose otp",
expectStdOut: "" +
"Available MFA methods [WEBAUTHN, SSO, OTP]. Continuing with WEBAUTHN and OTP.\n" +
Expand All @@ -255,28 +266,32 @@ func TestCLIPrompt(t *testing.T) {
},
},
},
}, {
},
{
name: "NOK no webauthn response",
expectStdOut: "Tap any security key\n",
challenge: &proto.MFAAuthenticateChallenge{
WebauthnChallenge: &webauthnpb.CredentialAssertion{},
},
expectErr: context.DeadlineExceeded,
}, {
},
{
name: "NOK no sso response",
expectStdOut: "",
challenge: &proto.MFAAuthenticateChallenge{
SSOChallenge: &proto.SSOChallenge{},
},
expectErr: context.DeadlineExceeded,
}, {
},
{
name: "NOK no otp response",
expectStdOut: "Enter an OTP code from a device:\n",
challenge: &proto.MFAAuthenticateChallenge{
TOTP: &proto.TOTPChallenge{},
},
expectErr: context.DeadlineExceeded,
}, {
},
{
name: "NOK no webauthn or otp response",
expectStdOut: "Tap any security key or enter a code from a OTP device\n",
challenge: &proto.MFAAuthenticateChallenge{
Expand Down Expand Up @@ -447,7 +462,7 @@ Enter your security key PIN:
tc.modifyPromptConfig(cliPromptConfig)
}

resp, err := mfa.NewCLIPromptV2(cliPromptConfig).Run(ctx, tc.challenge)
resp, err := mfa.NewCLIPrompt(cliPromptConfig).Run(ctx, tc.challenge)
if tc.expectErr != nil {
require.ErrorIs(t, err, tc.expectErr)
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/client/mfa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestPromptMFAChallenge_usingNonRegisteredDevice(t *testing.T) {
test.customizePrompt(cliConfig)
}

_, err := mfa.NewCLIPromptV2(cliConfig).Run(ctx, test.challenge)
_, err := mfa.NewCLIPrompt(cliConfig).Run(ctx, test.challenge)
if !errors.Is(err, wancli.ErrUsingNonRegisteredDevice) {
t.Errorf("PromptMFAChallenge returned err=%q, want %q", err, wancli.ErrUsingNonRegisteredDevice)
}
Expand Down
2 changes: 1 addition & 1 deletion tool/tctl/common/admin_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ func newAdminActionTestSuite(t *testing.T) *adminActionTestSuite {
promptCfg := libmfa.NewPromptConfig(proxyPublicAddr.String(), opts...)
promptCfg.WebauthnLoginFunc = mockWebauthnLogin
promptCfg.WebauthnSupported = true
return libmfa.NewCLIPromptV2(&libmfa.CLIPromptConfig{
return libmfa.NewCLIPrompt(&libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
})
}
Expand Down
2 changes: 1 addition & 1 deletion tool/tctl/common/tctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ 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.NewCLIPromptV2(&libmfa.CLIPromptConfig{
return libmfa.NewCLIPrompt(&libmfa.CLIPromptConfig{
PromptConfig: *promptCfg,
})
})
Expand Down

0 comments on commit f282b69

Please sign in to comment.