Adapt/backport tests and fix prefer OTP

gravitational · Jun 6, 2022 · efc7df8 · efc7df8
1 parent 951367c
commit efc7df8
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 33 deletions.
diff --git a/lib/client/api_login_test.go b/lib/client/api_login_test.go
@@ -159,46 +159,61 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) {
 		solveOTP         func(context.Context) (string, error)
 		solveU2F         func(ctx context.Context, facet string, challenges ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error)
 		solveWebauthn    func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error)
-		useStrongestAuth bool
+		allowStdinHijack bool
+		preferOTP        bool
 	}{
 		{
-			name:         "OK OTP device login",
+			name:         "OK OTP device login with hijack",
 			secondFactor: constants.SecondFactorOptional,
 			solveOTP:     solveOTP,
 			solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) {
 				panic("unused")
 			},
-			solveWebauthn: promptWebauthnNoop,
+			solveWebauthn:    promptWebauthnNoop,
+			allowStdinHijack: true,
 		},
 		{
-			name:         "OK Webauthn device login",
+			name:         "OK Webauthn device login with hijack",
 			secondFactor: constants.SecondFactorOptional,
 			solveOTP:     promptOTPNoop,
 			solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) {
 				panic("unused")
 			},
-			solveWebauthn: solveWebauthn,
+			solveWebauthn:    solveWebauthn,
+			allowStdinHijack: true,
 		},
 		{
-			name:         "Webauthn and UseStrongestAuth",
-			secondFactor: constants.SecondFactorOptional,
-			solveOTP: func(ctx context.Context) (string, error) {
+			name:         "OK U2F device login with hijack",
+			secondFactor: constants.SecondFactorU2F,
+			solveOTP:     promptOTPNoop,
+			solveU2F:     solveU2F,
+			solveWebauthn: func(context.Context, string, *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) {
 				panic("unused")
 			},
+			allowStdinHijack: true,
+		},
+		{
+			name:         "OTP preferred",
+			secondFactor: constants.SecondFactorOptional,
+			solveOTP:     solveOTP,
 			solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) {
 				panic("unused")
 			},
-			solveWebauthn:    solveWebauthn,
-			useStrongestAuth: true,
+			solveWebauthn: func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) {
+				panic("unused")
+			},
+			preferOTP: true,
 		},
 		{
-			name:         "OK U2F device login",
-			secondFactor: constants.SecondFactorU2F,
-			solveOTP:     promptOTPNoop,
-			solveU2F:     solveU2F,
-			solveWebauthn: func(context.Context, string, *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) {
+			name:         "Webauthn device login",
+			secondFactor: constants.SecondFactorOptional,
+			solveOTP: func(ctx context.Context) (string, error) {
+				panic("unused")
+			},
+			solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) {
 				panic("unused")
 			},
+			solveWebauthn: solveWebauthn,
 		},
 	}
 	for _, test := range tests {
@@ -222,7 +237,8 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) {
 
 			tc, err := client.NewClient(cfg)
 			require.NoError(t, err)
-			tc.UseStrongestAuth = test.useStrongestAuth
+			tc.AllowStdinHijack = test.allowStdinHijack
+			tc.PreferOTP = test.preferOTP
 
 			clock.Advance(30 * time.Second)
 			_, err = tc.Login(ctx)

diff --git a/lib/client/mfa.go b/lib/client/mfa.go
@@ -159,23 +159,25 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge,
 	}
 
 	// Fire Webauthn or U2F goroutine.
-	origin := proxyAddr
-	if !strings.HasPrefix(origin, "https://") {
-		origin = "https://" + origin
-	}
-	switch {
-	case c.WebauthnChallenge != nil:
-		go func() {
-			log.Debugf("WebAuthn: prompting U2F devices with origin %q", origin)
-			resp, err := promptWebauthn(ctx, origin, wanlib.CredentialAssertionFromProto(c.WebauthnChallenge))
-			respC <- response{kind: "WEBAUTHN", resp: resp, err: err}
-		}()
-	case len(c.U2F) > 0:
-		go func() {
-			log.Debugf("prompting U2F devices with facet %q", origin)
-			resp, err := promptU2FChallenges(ctx, proxyAddr, c.U2F)
-			respC <- response{kind: "U2F", resp: resp, err: err}
-		}()
+	if hasNonTOTP {
+		origin := proxyAddr
+		if !strings.HasPrefix(origin, "https://") {
+			origin = "https://" + origin
+		}
+		switch {
+		case c.WebauthnChallenge != nil:
+			go func() {
+				log.Debugf("WebAuthn: prompting U2F devices with origin %q", origin)
+				resp, err := promptWebauthn(ctx, origin, wanlib.CredentialAssertionFromProto(c.WebauthnChallenge))
+				respC <- response{kind: "WEBAUTHN", resp: resp, err: err}
+			}()
+		case len(c.U2F) > 0:
+			go func() {
+				log.Debugf("prompting U2F devices with facet %q", origin)
+				resp, err := promptU2FChallenges(ctx, proxyAddr, c.U2F)
+				respC <- response{kind: "U2F", resp: resp, err: err}
+			}()
+		}
 	}
 
 	for i := 0; i < numGoroutines; i++ {