Avoid invoking token helper on login

`vault login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `vault login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes openbao#312.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>

command/base.go

@@ -73,9 +73,10 @@ type BaseCommand struct {
 	client *api.Client
 }
 
-// Client returns the HTTP API client. The client is cached on the command to
-// save performance on future calls.
-func (c *BaseCommand) Client() (*api.Client, error) {
+// Construct the HTTP API client, but do not set the token on it yet. This is to
+// avoid invoking the token helper for calls that do not need a token, such as
+// `vault login`.
+func (c *BaseCommand) ClientWithoutToken() (*api.Client, error) {
 	// Read the test client if present
 	if c.client != nil {
 		return c.client, nil
@@ -133,26 +134,6 @@ func (c *BaseCommand) Client() (*api.Client, error) {
 	// Set the wrapping function
 	client.SetWrappingLookupFunc(c.DefaultWrappingLookupFunc)
 
-	// Get the token if it came in from the environment
-	token := client.Token()
-
-	// If we don't have a token, check the token helper
-	if token == "" {
-		helper, err := c.TokenHelper()
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to get token helper")
-		}
-		token, err = helper.Get()
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to get token from token helper")
-		}
-	}
-
-	// Set the token
-	if token != "" {
-		client.SetToken(token)
-	}
-
 	client.SetMFACreds(c.flagMFA)
 
 	// flagNS takes precedence over flagNamespace. After resolution, point both
@@ -184,6 +165,42 @@ func (c *BaseCommand) Client() (*api.Client, error) {
 		}
 	}
 
+	return client, nil
+}
+
+// Client returns the HTTP API client. The client is cached on the command to
+// save performance on future calls.
+func (c *BaseCommand) Client() (*api.Client, error) {
+	// Read the test client if present
+	if c.client != nil {
+		return c.client, nil
+	}
+
+	client, err := c.ClientWithoutToken()
+	if err != nil {
+		return nil, err
+	}
+
+	// Get the token if it came in from the environment
+	token := client.Token()
+
+	// If we don't have a token, check the token helper
+	if token == "" {
+		helper, err := c.TokenHelper()
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to get token helper")
+		}
+		token, err = helper.Get()
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to get token from token helper")
+		}
+	}
+
+	// Set the token
+	if token != "" {
+		client.SetToken(token)
+	}
+
 	c.client = client
 
 	return client, nil

command/login.go

@@ -216,7 +216,7 @@ func (c *LoginCommand) Run(args []string) int {
 	}
 
 	// Create the client
-	client, err := c.Client()
+	client, err := c.ClientWithoutToken()
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 2