VAULT-2285 adding capability to accept comma separated entries for au…

changelog/12126.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+cli/api: Providing consistency for the use of comma separated parameters in auth/secret enable/tune
+```

command/auth_enable_test.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/helper/builtinplugins"
 	"github.com/hashicorp/vault/sdk/helper/consts"
 	"github.com/mitchellh/cli"
@@ -86,6 +87,12 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 		code := cmd.Run([]string{
 			"-path", "auth_integration/",
 			"-description", "The best kind of test",
+			"-audit-non-hmac-request-keys", "foo,bar",
+			"-audit-non-hmac-response-keys", "foo,bar",
+			"-passthrough-request-headers", "authorization,authentication",
+			"-passthrough-request-headers", "www-authentication",
+			"-allowed-response-headers", "authorization",
+			"-listing-visibility", "unauth",
 			"userpass",
 		})
 		if exp := 0; code != exp {
@@ -113,6 +120,18 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 		if exp := "The best kind of test"; authInfo.Description != exp {
 			t.Errorf("expected %q to be %q", authInfo.Description, exp)
 		}
+		if diff := deep.Equal([]string{"authorization,authentication", "www-authentication"}, authInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"authorization"}, authInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"foo,bar"}, authInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"foo,bar"}, authInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
+		}
 	})
 
 	t.Run("communication_failure", func(t *testing.T) {

command/auth_tune.go

@@ -20,15 +20,17 @@ var (
 type AuthTuneCommand struct {
 	*BaseCommand
 
-	flagAuditNonHMACRequestKeys  []string
-	flagAuditNonHMACResponseKeys []string
-	flagDefaultLeaseTTL          time.Duration
-	flagDescription              string
-	flagListingVisibility        string
-	flagMaxLeaseTTL              time.Duration
-	flagOptions                  map[string]string
-	flagTokenType                string
-	flagVersion                  int
+	flagAuditNonHMACRequestKeys   []string
+	flagAuditNonHMACResponseKeys  []string
+	flagDefaultLeaseTTL           time.Duration
+	flagDescription               string
+	flagListingVisibility         string
+	flagMaxLeaseTTL               time.Duration
+	flagPassthroughRequestHeaders []string
+	flagAllowedResponseHeaders    []string
+	flagOptions                   map[string]string
+	flagTokenType                 string
+	flagVersion                   int
 }
 
 func (c *AuthTuneCommand) Synopsis() string {
@@ -107,6 +109,20 @@ func (c *AuthTuneCommand) Flags() *FlagSets {
 			"or a previously configured value for the auth method.",
 	})
 
+	f.StringSliceVar(&StringSliceVar{
+		Name:   flagNamePassthroughRequestHeaders,
+		Target: &c.flagPassthroughRequestHeaders,
+		Usage: "Comma-separated string or list of request header values that " +
+			"will be sent to the plugin",
+	})
+
+	f.StringSliceVar(&StringSliceVar{
+		Name:   flagNameAllowedResponseHeaders,
+		Target: &c.flagAllowedResponseHeaders,
+		Usage: "Comma-separated string or list of response header values that " +
+			"plugins will be allowed to set",
+	})
+
 	f.StringMapVar(&StringMapVar{
 		Name:       "options",
 		Target:     &c.flagOptions,
@@ -194,6 +210,14 @@ func (c *AuthTuneCommand) Run(args []string) int {
 			mountConfigInput.ListingVisibility = c.flagListingVisibility
 		}
 
+		if fl.Name == flagNamePassthroughRequestHeaders {
+			mountConfigInput.PassthroughRequestHeaders = c.flagPassthroughRequestHeaders
+		}
+
+		if fl.Name == flagNameAllowedResponseHeaders {
+			mountConfigInput.AllowedResponseHeaders = c.flagAllowedResponseHeaders
+		}
+
 		if fl.Name == flagNameTokenType {
 			mountConfigInput.TokenType = c.flagTokenType
 		}

command/auth_tune_test.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/api"
 	"github.com/mitchellh/cli"
 )
@@ -92,6 +93,9 @@ func TestAuthTuneCommand_Run(t *testing.T) {
 				"-max-lease-ttl", "1h",
 				"-audit-non-hmac-request-keys", "foo,bar",
 				"-audit-non-hmac-response-keys", "foo,bar",
+				"-passthrough-request-headers", "authorization",
+				"-passthrough-request-headers", "www-authentication",
+				"-allowed-response-headers", "authorization,www-authentication",
 				"-listing-visibility", "unauth",
 				"my-auth/",
 			})
@@ -126,6 +130,18 @@ func TestAuthTuneCommand_Run(t *testing.T) {
 			if exp := 3600; mountInfo.Config.MaxLeaseTTL != exp {
 				t.Errorf("expected %d to be %d", mountInfo.Config.MaxLeaseTTL, exp)
 			}
+			if diff := deep.Equal([]string{"authorization", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"authorization,www-authentication"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
+			}
 		})
 
 		t.Run("flags_description", func(t *testing.T) {

command/secrets_enable_test.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/helper/builtinplugins"
 	"github.com/hashicorp/vault/sdk/helper/consts"
 	"github.com/mitchellh/cli"
@@ -107,6 +108,11 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
 			"-description", "The best kind of test",
 			"-default-lease-ttl", "30m",
 			"-max-lease-ttl", "1h",
+			"-audit-non-hmac-request-keys", "foo,bar",
+			"-audit-non-hmac-response-keys", "foo,bar",
+			"-passthrough-request-headers", "authorization,authentication",
+			"-passthrough-request-headers", "www-authentication",
+			"-allowed-response-headers", "authorization",
 			"-force-no-cache",
 			"pki",
 		})
@@ -144,6 +150,19 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
 		if exp := true; mountInfo.Config.ForceNoCache != exp {
 			t.Errorf("expected %t to be %t", mountInfo.Config.ForceNoCache, exp)
 		}
+		if diff := deep.Equal([]string{"authorization,authentication", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"authorization"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
+		}
+		if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
+			t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
+		}
+
 	})
 
 	t.Run("communication_failure", func(t *testing.T) {

command/secrets_tune.go

@@ -20,14 +20,16 @@ var (
 type SecretsTuneCommand struct {
 	*BaseCommand
 
-	flagAuditNonHMACRequestKeys  []string
-	flagAuditNonHMACResponseKeys []string
-	flagDefaultLeaseTTL          time.Duration
-	flagDescription              string
-	flagListingVisibility        string
-	flagMaxLeaseTTL              time.Duration
-	flagOptions                  map[string]string
-	flagVersion                  int
+	flagAuditNonHMACRequestKeys   []string
+	flagAuditNonHMACResponseKeys  []string
+	flagDefaultLeaseTTL           time.Duration
+	flagDescription               string
+	flagListingVisibility         string
+	flagMaxLeaseTTL               time.Duration
+	flagPassthroughRequestHeaders []string
+	flagAllowedResponseHeaders    []string
+	flagOptions                   map[string]string
+	flagVersion                   int
 }
 
 func (c *SecretsTuneCommand) Synopsis() string {
@@ -106,6 +108,20 @@ func (c *SecretsTuneCommand) Flags() *FlagSets {
 			"TTL, or a previously configured value for the secrets engine.",
 	})
 
+	f.StringSliceVar(&StringSliceVar{
+		Name:   flagNamePassthroughRequestHeaders,
+		Target: &c.flagPassthroughRequestHeaders,
+		Usage: "Comma-separated string or list of request header values that " +
+			"will be sent to the plugin",
+	})
+
+	f.StringSliceVar(&StringSliceVar{
+		Name:   flagNameAllowedResponseHeaders,
+		Target: &c.flagAllowedResponseHeaders,
+		Usage: "Comma-separated string or list of response header values that " +
+			"plugins will be allowed to set",
+	})
+
 	f.StringMapVar(&StringMapVar{
 		Name:       "options",
 		Target:     &c.flagOptions,
@@ -189,6 +205,14 @@ func (c *SecretsTuneCommand) Run(args []string) int {
 		if fl.Name == flagNameListingVisibility {
 			mountConfigInput.ListingVisibility = c.flagListingVisibility
 		}
+
+		if fl.Name == flagNamePassthroughRequestHeaders {
+			mountConfigInput.PassthroughRequestHeaders = c.flagPassthroughRequestHeaders
+		}
+
+		if fl.Name == flagNameAllowedResponseHeaders {
+			mountConfigInput.AllowedResponseHeaders = c.flagAllowedResponseHeaders
+		}
 	})
 
 	if err := client.Sys().TuneMount(mountPath, mountConfigInput); err != nil {

command/secrets_tune_test.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/api"
 	"github.com/mitchellh/cli"
 )
@@ -166,6 +167,9 @@ func TestSecretsTuneCommand_Run(t *testing.T) {
 				"-max-lease-ttl", "1h",
 				"-audit-non-hmac-request-keys", "foo,bar",
 				"-audit-non-hmac-response-keys", "foo,bar",
+				"-passthrough-request-headers", "authorization",
+				"-passthrough-request-headers", "www-authentication",
+				"-allowed-response-headers", "authorization,www-authentication",
 				"-listing-visibility", "unauth",
 				"mount_tune_integration/",
 			})
@@ -200,6 +204,18 @@ func TestSecretsTuneCommand_Run(t *testing.T) {
 			if exp := 3600; mountInfo.Config.MaxLeaseTTL != exp {
 				t.Errorf("expected %d to be %d", mountInfo.Config.MaxLeaseTTL, exp)
 			}
+			if diff := deep.Equal([]string{"authorization", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
+				t.Errorf("Failed to find expected values for PassthroughRequestHeaders. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"authorization,www-authentication"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
+			}
+			if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
+				t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
+			}
 		})
 
 		t.Run("flags_description", func(t *testing.T) {

vault/logical_system.go

@@ -898,6 +898,13 @@ func (b *SystemBackend) handleMount(ctx context.Context, req *logical.Request, d
 	var apiConfig APIMountConfig
 
 	configMap := data.Get("config").(map[string]interface{})
+	// Augmenting configMap for some config options to treat them as comma separated entries
+	err := expandStringValsWithCommas(configMap)
+	if err != nil {
+		return logical.ErrorResponse(
+				"unable to parse given auth config information"),
+			logical.ErrInvalidRequest
+	}
 	if configMap != nil && len(configMap) != 0 {
 		err := mapstructure.Decode(configMap, &apiConfig)
 		if err != nil {
@@ -1564,7 +1571,6 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string,
 
 	if rawVal, ok := data.GetOk("allowed_response_headers"); ok {
 		headers := rawVal.([]string)
-
 		oldVal := mountEntry.Config.AllowedResponseHeaders
 		mountEntry.Config.AllowedResponseHeaders = headers
 
@@ -1869,6 +1875,31 @@ func (b *SystemBackend) handleAuthTable(ctx context.Context, req *logical.Reques
 	return resp, nil
 }
 
+func expandStringValsWithCommas(configMap map[string]interface{}) error {
+	configParamNameSlice := []string{
+		"audit_non_hmac_request_keys",
+		"audit_non_hmac_response_keys",
+		"passthrough_request_headers",
+		"allowed_response_headers",
+	}
+	for _, paramName := range configParamNameSlice {
+		if raw, ok := configMap[paramName]; ok {
+			switch t := raw.(type) {
+			case string:
+				// To be consistent with auth tune, and in cases where a single comma separated strings
+				// is provided in the curl command, we split the entries by the commas.
+				rawNew := raw.(string)
+				res, err := parseutil.ParseCommaStringSlice(rawNew)
+				if err != nil {
+					return fmt.Errorf("invalid input parameter %v of type %v", paramName, t)
+				}
+				configMap[paramName] = res
+			}
+		}
+	}
+	return nil
+}
+
 // handleEnableAuth is used to enable a new credential backend
 func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	repState := b.Core.ReplicationState()
@@ -1895,6 +1926,13 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque
 	var apiConfig APIMountConfig
 
 	configMap := data.Get("config").(map[string]interface{})
+	// Augmenting configMap for some config options to treat them as comma separated entries
+	err := expandStringValsWithCommas(configMap)
+	if err != nil {
+		return logical.ErrorResponse(
+				"unable to parse given auth config information"),
+			logical.ErrInvalidRequest
+	}
 	if configMap != nil && len(configMap) != 0 {
 		err := mapstructure.Decode(configMap, &apiConfig)
 		if err != nil {