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

[v17] Replace NewCLIPromptV2 #48187

Merged
merged 1 commit into from
Oct 31, 2024
Merged
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
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
Loading