Make tctl bots add prompt for MFA just once

api/mfa/ceremony.go

@@ -36,8 +36,7 @@ type MFACeremonyClient interface {
 
 // PerformMFACeremony retrieves an MFA challenge from the server with the given challenge extensions
 // and prompts the user to answer the challenge with the given promptOpts, and ultimately returning
-// an MFA challenge response for the user. A nil response will be returned if an MFA required check
-// is provided and MFA is not required.
+// an MFA challenge response for the user.
 func PerformMFACeremony(ctx context.Context, clt MFACeremonyClient, challengeRequest *proto.CreateAuthenticateChallengeRequest, promptOpts ...PromptOpt) (*proto.MFAAuthenticateResponse, error) {
 	if challengeRequest == nil {
 		return nil, trace.BadParameter("missing challenge request")
@@ -53,21 +52,26 @@ func PerformMFACeremony(ctx context.Context, clt MFACeremonyClient, challengeReq
 
 	chal, err := clt.CreateAuthenticateChallenge(ctx, challengeRequest)
 	if err != nil {
+		// CreateAuthenticateChallenge returns a bad parameter error when the the client
+		// user is not a Teleport user. For example, the AdminRole. Treat this as a false
+		// MFA required check if a check was requested.
+		if trace.IsBadParameter(err) {
+			return nil, &ErrMFANotRequired
+		}
 		return nil, trace.Wrap(err)
 	}
 
 	// If an MFA required check was provided, and the client discovers MFA is not required,
 	// skip the MFA prompt and return an empty response.
 	if chal.MFARequired == proto.MFARequired_MFA_REQUIRED_NO {
-		return nil, nil
+		return nil, &ErrMFANotRequired
 	}
 
 	return clt.PromptMFA(ctx, chal, promptOpts...)
 }
 
 // PerformAdminActionMFACeremony retrieves an MFA challenge from the server for an admin
 // action, prompts the user to answer the challenge, and returns the resulting MFA response.
-// An empty response will be returned if MFA is not required for the given admin action.
 func PerformAdminActionMFACeremony(ctx context.Context, clt MFACeremonyClient, allowReuse bool) (*proto.MFAAuthenticateResponse, error) {
 	allowReuseExt := mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO
 	if allowReuse {

api/mfa/ceremony_test.go

@@ -61,7 +61,7 @@ func TestPerformMFACeremony(t *testing.T) {
 				mfaRequired:       proto.MFARequired_MFA_REQUIRED_NO,
 			},
 			assertCeremonyResponse: func(t *testing.T, mr *proto.MFAAuthenticateResponse, err error, i ...interface{}) {
-				assert.NoError(t, err)
+				assert.Error(t, err, mfa.ErrMFANotRequired)
 				assert.Nil(t, mr)
 			},
 		}, {

api/mfa/mfa.go

@@ -33,9 +33,16 @@ import (
 // ResponseMetadataKey is the context metadata key for an MFA response in a gRPC request.
 const ResponseMetadataKey = "mfa_challenge_response"
 
-// ErrAdminActionMFARequired is an error indicating that an admin-level
-// API request failed due to missing MFA verification.
-var ErrAdminActionMFARequired = trace.AccessDeniedError{Message: "admin-level API request requires MFA verification"}
+var (
+	// ErrAdminActionMFARequired is an error indicating that an admin-level
+	// API request failed due to missing MFA verification.
+	ErrAdminActionMFARequired = trace.AccessDeniedError{Message: "admin-level API request requires MFA verification"}
+
+	// ErrMFANotRequired is returned by MFA ceremonies when it is discovered or
+	// inferred that the MFA ceremony is not necessary. This is usually because
+	// the server does require/support MFA for the user.
+	ErrMFANotRequired = trace.BadParameterError{Message: "re-authentication with MFA is not required"}
+)
 
 // WithCredentials can be called on a GRPC client request to attach
 // MFA credentials to the GRPC metadata for requests that require MFA,

tool/tctl/common/admin_action_test.go

@@ -995,6 +995,12 @@ func newAdminActionTestSuite(t *testing.T) *adminActionTestSuite {
 		Credentials: []client.Credentials{
 			client.LoadProfile(tshHome, ""),
 		},
+		MFAPromptConstructor: func(po ...mfa.PromptOpt) mfa.Prompt {
+			return mfa.PromptFunc(func(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
+				// return MFA not required to gracefully skip the MFA prompt.
+				return nil, &mfa.ErrMFANotRequired
+			})
+		},
 	})
 	require.NoError(t, err)
 

tool/tctl/common/bots_command.go

@@ -21,6 +21,7 @@ package common
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"maps"
 	"os"
@@ -39,6 +40,7 @@ import (
 	"github.com/gravitational/teleport/api/constants"
 	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
 	machineidv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1"
+	"github.com/gravitational/teleport/api/mfa"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/asciitable"
 	"github.com/gravitational/teleport/lib/auth"
@@ -300,6 +302,14 @@ func (c *BotsCommand) addBotLegacy(ctx context.Context, client auth.ClientI) err
 
 // AddBot adds a new certificate renewal bot to the cluster.
 func (c *BotsCommand) AddBot(ctx context.Context, client auth.ClientI) error {
+	// Prompt for admin action MFA if required, allowing reuse for UpsertToken and CreateBot.
+	mfaResponse, err := mfa.PerformAdminActionMFACeremony(ctx, client, true /*allowReuse*/)
+	if err == nil {
+		ctx = mfa.ContextWithMFAResponse(ctx, mfaResponse)
+	} else if !errors.Is(err, &mfa.ErrMFANotRequired) {
+		return trace.Wrap(err)
+	}
+
 	// Jankily call the endpoint invalidly. This lets us version check and use
 	// the legacy version of this CLI tool if we are talking to an older
 	// server.
@@ -318,7 +328,6 @@ func (c *BotsCommand) AddBot(ctx context.Context, client auth.ClientI) error {
 		log.Warning("No roles specified. The bot will not be able to produce outputs until a role is added to the bot.")
 	}
 	var token types.ProvisionToken
-	var err error
 	if c.tokenID == "" {
 		// If there's no token specified, generate one
 		tokenName, err := utils.CryptoRandomHex(16)

tool/tctl/common/user_command.go

@@ -21,6 +21,7 @@ package common
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -299,13 +300,11 @@ func (u *UserCommand) Add(ctx context.Context, client auth.ClientI) error {
 	user.SetRoles(u.allowedRoles)
 
 	// Prompt for admin action MFA if required, allowing reuse for CreateResetPasswordToken.
-	if u.config.Auth.Preference.IsAdminActionMFAEnforced() {
-		mfaResponse, err := mfa.PerformAdminActionMFACeremony(ctx, client, true /*allowReuse*/)
-		if err != nil {
-			return trace.Wrap(err)
-		} else if mfaResponse != nil {
-			ctx = mfa.ContextWithMFAResponse(ctx, mfaResponse)
-		}
+	mfaResponse, err := mfa.PerformAdminActionMFACeremony(ctx, client, true /*allowReuse*/)
+	if err == nil {
+		ctx = mfa.ContextWithMFAResponse(ctx, mfaResponse)
+	} else if !errors.Is(err, &mfa.ErrMFANotRequired) {
+		return trace.Wrap(err)
 	}
 
 	if _, err := client.CreateUser(ctx, user); err != nil {