From 9b4ce696587850b33b3b2a52407d4e17d588f6b1 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 23 Oct 2024 13:42:10 -0700 Subject: [PATCH] Add --mfa-mode=sso support; Add cli prompt UX changes. --- lib/client/api.go | 3 + lib/client/mfa.go | 1 + lib/client/mfa/cli.go | 73 ++++++++-- lib/client/mfa/cli_test.go | 242 +++++++++++++++++++++++++++++--- lib/client/sso/ceremony_test.go | 2 +- tool/tsh/common/tsh.go | 8 +- 6 files changed, 298 insertions(+), 31 deletions(-) diff --git a/lib/client/api.go b/lib/client/api.go index a03cdfd443a94..73c9883a115cb 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -360,6 +360,9 @@ type Config struct { // authenticators, such as remote hosts or virtual machines. PreferOTP bool + // PreferSSO prefers SSO in favor of other MFA methods. + PreferSSO bool + // CheckVersions will check that client version is compatible // with auth server version when connecting. CheckVersions bool diff --git a/lib/client/mfa.go b/lib/client/mfa.go index b3431cebd5937..b5a7c8729141e 100644 --- a/lib/client/mfa.go +++ b/lib/client/mfa.go @@ -63,6 +63,7 @@ func (tc *TeleportClient) NewMFAPrompt(opts ...mfa.PromptOpt) mfa.Prompt { PromptConfig: *cfg, Writer: tc.Stderr, PreferOTP: tc.PreferOTP, + PreferSSO: tc.PreferSSO, AllowStdinHijack: tc.AllowStdinHijack, StdinFunc: tc.StdinFunc, }) diff --git a/lib/client/mfa/cli.go b/lib/client/mfa/cli.go index e44676b001fe8..8f643ec1122b2 100644 --- a/lib/client/mfa/cli.go +++ b/lib/client/mfa/cli.go @@ -25,6 +25,7 @@ import ( "log/slog" "os" "runtime" + "strings" "sync" "github.com/gravitational/trace" @@ -37,6 +38,15 @@ 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" + // cliMFATypeSSO is the CLI display name for SSO. + cliMFATypeSSO = "SSO" +) + // CLIPromptConfig contains CLI prompt config options. type CLIPromptConfig struct { PromptConfig @@ -51,6 +61,9 @@ type CLIPromptConfig 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 // StdinFunc allows tests to override prompt.Stdin(). // If nil prompt.Stdin() is used. StdinFunc func() prompt.StdinReader @@ -112,17 +125,25 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng promptSSO := chal.SSOChallenge != nil // No prompt to run, no-op. - if !promptOTP && !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) + } + // Check off unsupported methods. if promptWebauthn && !c.cfg.WebauthnSupported { promptWebauthn = false slog.DebugContext(ctx, "hardware device MFA not supported by your platform") - if !promptOTP { - return nil, trace.BadParameter("hardware device MFA not supported by your platform, please register an OTP device") - } } if promptSSO && c.cfg.SSOMFACeremony == nil { @@ -131,8 +152,17 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng } // Prefer whatever method is requested by the client. - if c.cfg.PreferOTP && promptOTP { - promptWebauthn = false + var chosenMethods []string + var userSpecifiedMethod bool + switch { + case c.cfg.PreferSSO && promptSSO: + chosenMethods = []string{cliMFATypeSSO} + promptWebauthn, promptOTP = false, false + userSpecifiedMethod = true + case c.cfg.PreferOTP && promptOTP: + chosenMethods = []string{cliMFATypeOTP} + promptWebauthn, promptSSO = false, false + userSpecifiedMethod = true } // Use stronger auth methods if hijack is not allowed. @@ -140,10 +170,32 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng promptOTP = false } - // If a specific webauthn attachment was requested, skip OTP. - // Otherwise, allow dual prompt with OTP. - if promptWebauthn && c.cfg.AuthenticatorAttachment != wancli.AttachmentAuto { + // If we have multiple viable options, prefer Webauthn > SSO > OTP. + switch { + case promptWebauthn: + chosenMethods = []string{cliMFATypeWebauthn} + promptSSO = false + + // If a specific webauthn attachment was requested, skip OTP. + // Otherwise, allow dual prompt with OTP. + promptOTP = promptOTP && c.cfg.AuthenticatorAttachment == wancli.AttachmentAuto + if promptOTP { + chosenMethods = append(chosenMethods, cliMFATypeOTP) + } + case promptSSO: + chosenMethods = []string{cliMFATypeSSO} promptOTP = false + case promptOTP: + chosenMethods = []string{cliMFATypeOTP} + } + + // If there are multiple options and we chose one without it being specifically + // requested by the user, notify the user about it and how to request a specific method. + if len(availableMethods) > len(chosenMethods) && len(chosenMethods) > 0 && !userSpecifiedMethod { + const msg = "" + + "Available MFA methods [%v]. Continuing with %v.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + fmt.Fprintf(c.writer(), msg, strings.Join(availableMethods, ", "), strings.Join(chosenMethods, " and ")) } switch { @@ -160,8 +212,7 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng 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. - return nil, trace.BadParameter("no MFA methods to prompt") + return nil, trace.BadParameter("client does not support any available MFA methods [%v], see debug logs for details", strings.Join(availableMethods, ", ")) } } diff --git a/lib/client/mfa/cli_test.go b/lib/client/mfa/cli_test.go index 54e0fcfd92fd9..683932fb7c47b 100644 --- a/lib/client/mfa/cli_test.go +++ b/lib/client/mfa/cli_test.go @@ -43,6 +43,7 @@ func TestCLIPrompt(t *testing.T) { name string stdin string challenge *proto.MFAAuthenticateChallenge + modifyPromptConfig func(cfg *mfa.CLIPromptConfig) expectErr error expectStdOut string expectResp *proto.MFAAuthenticateResponse @@ -65,7 +66,7 @@ func TestCLIPrompt(t *testing.T) { }, }, }, { - name: "OK totp", + name: "OK otp", expectStdOut: "Enter an OTP code from a device:\n", stdin: "123456", challenge: &proto.MFAAuthenticateChallenge{ @@ -79,11 +80,86 @@ func TestCLIPrompt(t *testing.T) { }, }, }, { - name: "OK webauthn or totp choose webauthn", - expectStdOut: "Tap any security key or enter a code from a OTP device\n", + name: "OK sso", + expectStdOut: "", // sso stdout is handled internally in the SSO ceremony, which is mocked in this test. + challenge: &proto.MFAAuthenticateChallenge{ + SSOChallenge: &proto.SSOChallenge{}, + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_SSO{ + SSO: &proto.SSOResponse{ + RequestId: "request-id", + Token: "mfa-token", + }, + }, + }, + }, { + name: "OK prefer otp when specified", + expectStdOut: "Enter an OTP code from a device:\n", + stdin: "123456", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.PreferOTP = true + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_TOTP{ + TOTP: &proto.TOTPResponse{ + Code: "123456", + }, + }, + }, + }, { + name: "OK prefer sso when specified", + expectStdOut: "", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.PreferSSO = true + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_SSO{ + SSO: &proto.SSOResponse{ + RequestId: "request-id", + Token: "mfa-token", + }, + }, + }, + }, { + name: "OK prefer webauthn with authenticator attachment requested", + expectStdOut: "" + + "Available MFA methods [WEBAUTHN, SSO, OTP]. Continuing with WEBAUTHN.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key\n", challenge: &proto.MFAAuthenticateChallenge{ WebauthnChallenge: &webauthnpb.CredentialAssertion{}, TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AuthenticatorAttachment = wancli.AttachmentPlatform + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_Webauthn{ + Webauthn: &webauthnpb.CredentialAssertionResponse{}, + }, + }, + }, + { + name: "OK prefer webauthn over sso", + expectStdOut: "" + + "Available MFA methods [WEBAUTHN, SSO]. Continuing with WEBAUTHN.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key\n", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + SSOChallenge: &proto.SSOChallenge{}, }, expectResp: &proto.MFAAuthenticateResponse{ Response: &proto.MFAAuthenticateResponse_Webauthn{ @@ -91,13 +167,90 @@ func TestCLIPrompt(t *testing.T) { }, }, }, { - name: "OK webauthn or totp choose totp", - expectStdOut: "Tap any security key or enter a code from a OTP device\n", - stdin: "123456", + name: "OK prefer webauthn+otp over sso", + expectStdOut: "" + + "Available MFA methods [WEBAUTHN, SSO, OTP]. Continuing with WEBAUTHN and OTP.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key or enter a code from a OTP device\n", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_Webauthn{ + Webauthn: &webauthnpb.CredentialAssertionResponse{}, + }, + }, + }, { + name: "OK prefer sso over otp", + expectStdOut: "" + + "Available MFA methods [SSO, OTP]. Continuing with SSO.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n", + challenge: &proto.MFAAuthenticateChallenge{ + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_SSO{ + SSO: &proto.SSOResponse{ + RequestId: "request-id", + Token: "mfa-token", + }, + }, + }, + }, { + name: "OK prefer webauthn over otp when stdin hijack disallowed", + expectStdOut: "" + + "Available MFA methods [WEBAUTHN, OTP]. Continuing with WEBAUTHN.\n" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key\n", challenge: &proto.MFAAuthenticateChallenge{ WebauthnChallenge: &webauthnpb.CredentialAssertion{}, TOTP: &proto.TOTPChallenge{}, }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_Webauthn{ + 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" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key or enter a code from a OTP device\n", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, + expectResp: &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_Webauthn{ + 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" + + "If you wish to perform MFA with another method, specify with flag --mfa-mode=.\n\n" + + "Tap any security key or enter a code from a OTP device\n", + stdin: "123456", + challenge: &proto.MFAAuthenticateChallenge{ + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + TOTP: &proto.TOTPChallenge{}, + SSOChallenge: &proto.SSOChallenge{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, expectResp: &proto.MFAAuthenticateResponse{ Response: &proto.MFAAuthenticateResponse_TOTP{ TOTP: &proto.TOTPResponse{ @@ -113,19 +266,29 @@ func TestCLIPrompt(t *testing.T) { }, expectErr: context.DeadlineExceeded, }, { - name: "NOK no totp response", + 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 totp response", + name: "NOK no webauthn or otp response", expectStdOut: "Tap any security key or enter a code from a OTP device\n", challenge: &proto.MFAAuthenticateChallenge{ WebauthnChallenge: &webauthnpb.CredentialAssertion{}, TOTP: &proto.TOTPChallenge{}, }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, expectErr: context.DeadlineExceeded, }, { @@ -134,6 +297,9 @@ func TestCLIPrompt(t *testing.T) { TOTP: &proto.TOTPChallenge{}, WebauthnChallenge: &webauthnpb.CredentialAssertion{}, }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, expectStdOut: `Tap any security key or enter a code from a OTP device Detected security key tap Enter your security key PIN: @@ -185,6 +351,9 @@ Enter your security key PIN: TOTP: nil, // no TOTP challenge WebauthnChallenge: &webauthnpb.CredentialAssertion{}, }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.AllowStdinHijack = true + }, stdin: "1234", expectStdOut: `Tap any security key Detected security key tap @@ -224,19 +393,27 @@ Enter your security key PIN: } }, }, + { + name: "NOK webauthn and SSO not supported", + challenge: &proto.MFAAuthenticateChallenge{ + SSOChallenge: &proto.SSOChallenge{}, + WebauthnChallenge: &webauthnpb.CredentialAssertion{}, + }, + modifyPromptConfig: func(cfg *mfa.CLIPromptConfig) { + cfg.WebauthnSupported = false + cfg.SSOMFACeremony = nil + }, + expectErr: trace.BadParameter("client does not support any available MFA methods [WEBAUTHN, SSO], see debug logs for details"), + }, } { t.Run(tc.name, func(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) defer cancel() - oldStdin := prompt.Stdin() - t.Cleanup(func() { prompt.SetStdin(oldStdin) }) - stdin := prompt.NewFakeReader() if tc.stdin != "" { stdin.AddString(tc.stdin) } - prompt.SetStdin(stdin) cfg := mfa.NewPromptConfig("proxy.example.com") cfg.WebauthnSupported = true @@ -257,16 +434,26 @@ Enter your security key PIN: } } + cfg.SSOMFACeremony = &mockSSOMFACeremony{ + mfaResp: tc.expectResp, + } + buffer := make([]byte, 0, 100) out := bytes.NewBuffer(buffer) - prompt := mfa.NewCLIPromptV2(&mfa.CLIPromptConfig{ - PromptConfig: *cfg, - Writer: out, - AllowStdinHijack: true, - }) - resp, err := prompt.Run(ctx, tc.challenge) + cliPromptConfig := &mfa.CLIPromptConfig{ + PromptConfig: *cfg, + Writer: out, + StdinFunc: func() prompt.StdinReader { + return stdin + }, + } + if tc.modifyPromptConfig != nil { + tc.modifyPromptConfig(cliPromptConfig) + } + + resp, err := mfa.NewCLIPromptV2(cliPromptConfig).Run(ctx, tc.challenge) if tc.expectErr != nil { require.ErrorIs(t, err, tc.expectErr) } else { @@ -278,3 +465,22 @@ Enter your security key PIN: }) } } + +type mockSSOMFACeremony struct { + mfaResp *proto.MFAAuthenticateResponse +} + +func (m *mockSSOMFACeremony) GetClientCallbackURL() string { + return "" +} + +// Run the SSO MFA ceremony. +func (m *mockSSOMFACeremony) Run(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { + if m.mfaResp == nil { + return nil, context.DeadlineExceeded + } + if m.mfaResp.GetSSO() == nil { + return nil, trace.BadParameter("expected an SSO response but got %T", m.mfaResp.Response) + } + return m.mfaResp, nil +} diff --git a/lib/client/sso/ceremony_test.go b/lib/client/sso/ceremony_test.go index a2b15b532e54b..a914a8f76fe5a 100644 --- a/lib/client/sso/ceremony_test.go +++ b/lib/client/sso/ceremony_test.go @@ -28,8 +28,8 @@ import ( "testing" "github.com/gravitational/trace" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gotest.tools/assert" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index d7573c092a9d2..1375e455af9b4 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -117,6 +117,8 @@ const ( mfaModePlatform = "platform" // mfaModeOTP utilizes only OTP devices. mfaModeOTP = "otp" + // mfaModeSSO utilizes only SSO devices. + mfaModeSSO = "sso" ) const ( @@ -756,7 +758,7 @@ func Run(ctx context.Context, args []string, opts ...CliOption) error { app.Flag("bind-addr", "Override host:port used when opening a browser for cluster logins").Envar(bindAddrEnvVar).StringVar(&cf.BindAddr) app.Flag("callback", "Override the base URL (host:port) of the link shown when opening a browser for cluster logins. Must be used with --bind-addr.").StringVar(&cf.CallbackAddr) app.Flag("browser-login", browserHelp).Hidden().Envar(browserEnvVar).StringVar(&cf.Browser) - modes := []string{mfaModeAuto, mfaModeCrossPlatform, mfaModePlatform, mfaModeOTP} + modes := []string{mfaModeAuto, mfaModeCrossPlatform, mfaModePlatform, mfaModeOTP, mfaModeSSO} app.Flag("mfa-mode", fmt.Sprintf("Preferred mode for MFA and Passwordless assertions (%v)", strings.Join(modes, ", "))). Default(mfaModeAuto). Envar(mfaModeEnvVar). @@ -4210,6 +4212,7 @@ func loadClientConfigFromCLIConf(cf *CLIConf, proxy string) (*client.Config, err } c.AuthenticatorAttachment = mfaOpts.AuthenticatorAttachment c.PreferOTP = mfaOpts.PreferOTP + c.PreferSSO = mfaOpts.PreferSSO // If agent forwarding was specified on the command line enable it. c.ForwardAgent = options.ForwardAgent @@ -4391,6 +4394,7 @@ func (c *CLIConf) GetProfile() (*profile.Profile, error) { type mfaModeOpts struct { AuthenticatorAttachment wancli.AuthenticatorAttachment PreferOTP bool + PreferSSO bool } func parseMFAMode(mode string) (*mfaModeOpts, error) { @@ -4403,6 +4407,8 @@ func parseMFAMode(mode string) (*mfaModeOpts, error) { opts.AuthenticatorAttachment = wancli.AttachmentPlatform case mfaModeOTP: opts.PreferOTP = true + case mfaModeSSO: + opts.PreferSSO = true default: return nil, fmt.Errorf("invalid MFA mode: %q", mode) }