Respect WithWrappingToken for all secret ID's in approle auth (#13241)

api/auth/approle/approle.go

@@ -22,13 +22,12 @@ type AppRoleAuth struct {
 var _ api.AuthMethod = (*AppRoleAuth)(nil)
 
 // SecretID is a struct that allows you to specify where your application is
-// storing the secret ID required for login to the AppRole auth method.
+// storing the secret ID required for login to the AppRole auth method. The
+// recommended secure pattern is to use response-wrapping tokens rather than
+// a plaintext value, by passing WithWrappingToken() to NewAppRoleAuth.
+// https://learn.hashicorp.com/tutorials/vault/approle-best-practices?in=vault/auth-methods#secretid-delivery-best-practices
 type SecretID struct {
-	// Path on the file system where a trusted orchestrator has placed the
+	// Path on the file system where the secret ID can be found.
-	// application's secret ID. The recommended secure pattern is to use
-	// response-wrapping tokens rather than a plaintext value, by passing
-	// WithWrappingToken() to NewAppRoleAuth.
-	// https://learn.hashicorp.com/tutorials/vault/approle-best-practices?in=vault/auth-methods#secretid-delivery-best-practices
 	FromFile string
 	// The name of the environment variable containing the application's
 	// secret ID.
@@ -105,31 +104,34 @@ func (a *AppRoleAuth) Login(ctx context.Context, client *api.Client) (*api.Secre
 		"role_id": a.roleID,
 	}
 
-	if a.secretIDFile != "" {
+	var secretIDValue string
-		secretIDValue, err := a.readSecretIDFromFile()
+
+	switch {
+	case a.secretIDFile != "":
+		s, err := a.readSecretIDFromFile()
 		if err != nil {
 			return nil, fmt.Errorf("error reading secret ID: %w", err)
 		}
+		secretIDValue = s
+	case a.secretIDEnv != "":
+		s := os.Getenv(a.secretIDEnv)
+		if s == "" {
+			return nil, fmt.Errorf("secret ID was specified with an environment variable %q with an empty value", a.secretIDEnv)
+		}
+		secretIDValue = s
+	default:
+		secretIDValue = a.secretID
+	}
 
-		// if it was indicated that the value in the file was actually a wrapping
+	// if the caller indicated that the value was actually a wrapping token, unwrap it first
-		// token, unwrap it first
+	if a.unwrap {
-		if a.unwrap {
+		unwrappedToken, err := client.Logical().Unwrap(secretIDValue)
-			unwrappedToken, err := client.Logical().Unwrap(secretIDValue)
+		if err != nil {
-			if err != nil {
+			return nil, fmt.Errorf("unable to unwrap response wrapping token: %w", err)
-				return nil, fmt.Errorf("unable to unwrap token: %w. If the AppRoleAuth struct was initialized with the WithWrappingToken LoginOption, then the secret ID's filepath should be a path to a response-wrapping token", err)
-			}
-			loginData["secret_id"] = unwrappedToken.Data["secret_id"]
-		} else {
-			loginData["secret_id"] = secretIDValue
 		}
-	} else if a.secretIDEnv != "" {
+		loginData["secret_id"] = unwrappedToken.Data["secret_id"]
-		secretIDValue := os.Getenv(a.secretIDEnv)
-		if secretIDValue == "" {
-			return nil, fmt.Errorf("secret ID was specified with an environment variable with an empty value")
-		}
-		loginData["secret_id"] = secretIDValue
 	} else {
-		loginData["secret_id"] = a.secretID
+		loginData["secret_id"] = secretIDValue
 	}
 
 	path := fmt.Sprintf("auth/%s/login", a.mountPath)

changelog/13241.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+api: respect WithWrappingToken() option during AppRole login authentication when used with secret ID specified from environment or from string
+```