From 306ac83c96316152fe8bbde61bb2e6e98e449d33 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 25 Apr 2023 13:29:39 -0400 Subject: [PATCH] Avoid prompting users for mfa when using `tsh ssh --headless` (#24701) --- lib/client/api.go | 8 ++++++++ tool/tsh/tsh_test.go | 41 +++++++++++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/client/api.go b/lib/client/api.go index 64f38b0678804..2baa777fbf974 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -1607,6 +1607,14 @@ func (tc *TeleportClient) connectToNodeWithMFA(ctx context.Context, proxyClient ) defer span.End() + // There is no need to attempt a mfa ceremony if the user is attempting + // to connect to a resource via `tsh ssh --headless`. The entire point + // of headless authentication is to allow connections to originate from a + // machine without access to a WebAuthn device. + if tc.AuthConnector == constants.HeadlessConnector { + return nil, trace.Wrap(services.ErrSessionMFANotRequired) + } + if nodeDetails.MFACheck != nil && !nodeDetails.MFACheck.Required { return nil, trace.Wrap(services.ErrSessionMFANotRequired) } diff --git a/tool/tsh/tsh_test.go b/tool/tsh/tsh_test.go index af6dc26c81454..abd1def7828bd 100644 --- a/tool/tsh/tsh_test.go +++ b/tool/tsh/tsh_test.go @@ -936,8 +936,8 @@ func approveAllAccessRequests(ctx context.Context, approver accessApprover) erro // Sessions created via hostname and by matched labels are // verified. // -// NOTE: This test must NOT be run in parallel because it modifies the global -// [client.PromptWebauthn]. +// NOTE: This test must NOT be run in parallel because it updates +// the global [client.PromptWebauthn] in multiple test cases. func TestSSHOnMultipleNodes(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -1123,6 +1123,7 @@ func TestSSHOnMultipleNodes(t *testing.T) { errAssertion require.ErrorAssertionFunc stdoutAssertion require.ValueAssertionFunc mfaPromptCount int + headless bool }{ { name: "default auth preference runs commands on multiple nodes without mfa", @@ -1277,6 +1278,26 @@ func TestSSHOnMultipleNodes(t *testing.T) { mfaPromptCount: 1, errAssertion: require.Error, }, + { + name: "mfa ceremony prevented when using headless auth", + authPreference: &types.AuthPreferenceV2{ + Spec: types.AuthPreferenceSpecV2{ + Type: constants.Local, + SecondFactor: constants.SecondFactorOptional, + Webauthn: &types.Webauthn{ + RPID: "localhost", + }, + }, + }, + target: sshHostID, + roles: []string{perSessionMFARole.GetName()}, + setup: setupChallengeSolver(failedChallenge), + stdoutAssertion: func(t require.TestingT, i interface{}, i2 ...interface{}) { + require.Equal(t, "test\n", i, i2...) + }, + errAssertion: require.NoError, + headless: true, + }, } for _, tt := range cases { @@ -1322,16 +1343,20 @@ func TestSSHOnMultipleNodes(t *testing.T) { // Clear counter before each ssh command, // so we can assert how many times sign was called. device.SetCounter(0) - err = Run(ctx, []string{ - "ssh", - "--insecure", - tt.target, - "echo", "test", - }, + + args := []string{"ssh", "--insecure"} + if tt.headless { + args = append(args, "--headless", "--proxy", proxyAddr.String(), "--user", alice.GetName()) + } + args = append(args, tt.target, "echo", "test") + + err = Run(ctx, + args, setHomePath(tmpHomePath), func(conf *CLIConf) error { conf.overrideStdin = &bytes.Buffer{} conf.overrideStdout = stdout + conf.mockHeadlessLogin = mockHeadlessLogin(t, rootAuth.GetAuthServer(), alice) return nil }, )